hoodiehq / account-json-api Goto Github PK

Hey folks, I have a lot of questions about the API URLs and returned status codes, also, it seems to me that a lot of possible error responses are omitted from the API, so here is a compilations of questions and feedback about the current defined API for further discussion.

Disclaimer: I am focusing first on the "User" (i.e. non admin) part of the API, so this discussion is only about that part.

Session

Sign in PUT /session

Possible responses:

• session created: 201
• invalid credentials provided: 401
• wrong type provided in the request (data.type): 409*
• a user is logged in already (there is already an active session): ?

Check session GET /session

Possible responses:

• OK: 200
• invalid auth header: 401
• no active session: 401/404?**

Sign out DELETE /session

Possible responses:

• delete worked: 204
• invalid auth header: 401
• no active session: 401/404?**

Account

Sign up PUT /session/account

About the URL:

• shouldn't that be POST /accounts (this is the URL for admins)
• it seems weird to me to create a session using PUT /session/account when the session does not exist (since, if the session exist, then we already have an account for it, haven't we?)

Possible responses:

• account created: 201
• wrong type provided in the request (data.type): 409*
• a user is logged in already (there is already an active session, with a corresponding account): ?

Fetch user account GET /session/account

Possible responses:

• OK: 200
• invalid auth header: 401
• no active session: 401/404?**

Change user account PATCH /session/account

Possible responses:

• OK: 204
• invalid auth header: 401
• no active session: 401/404?**
• updating the user infringes some server-enforced constraints: 409 (this might not be relevant for us)
• type and id provided in the request don't match (data.type): 409***

Close account DELETE /session/account

Possible responses:

• OK: 204
• invalid auth header: 401
• no active session: 401/404?**

Profile

Fetch profile GET /session/account/profile

Possible responses:

• OK: 200
• no profile available: 200 with null as primary data. See JSON API spec.
• invalid auth header: 401
• no active session: 401/404?**

Create profile PUT /session/account/profile

Possible responses:

• profile created: 201
• invalid auth header: 401
• no active session: 401/404?**
• wrong type provided in the request (data.type): 409*
• a profile is available already: ?

Update profile PATCH /session/account/profile

Possible responses:

• OK: 204
• invalid auth header: 401
• no active session: 401/404?**
• updating the user infringes some server-enforced constraints: 409 (this might not be relevant for us)
• type and id provided in the request don't match (data.type): 409***

*This is mentioned in the JSON API spec referring to creating users with POST, I guess it also applies to PUT?

**Some considerations about this one:

• if there is no active session, then our auth credentials are not the right one, so response should be 401?
• 404 can be interpreted as: URL doesn't exist, but if the URL doesn't exist, we can't really use PUT for the creation of the resource, can we?

*** See here.

@gr2m, @tthew, @karlwestin comments?

GET requests without a JSON payload/body do not require a Content-Type header since no content is being sent to the server. This should be removed from the documentation.

Follow up for #14

As an admin, I would like to be able to create a user session, so I can debug an account without messing with passwords.

I would suggest the route POST /account/{id}/sessions for that. Without any body (does JSON API allow that?).

The response would look like this:

{
  "links": {
    "self": "https://example.com/accounts/abc1234/sessions/sessionid123"
  },
  "data": {
    "id": "sessionid123",
    "type": "session",
    "relationships": {
      "account": {
        "links": {
          "related": "https://example.com/accounts/abc1234"
        },
        "data": {
          "id": "abc1234",
          "type": "account"
        }
      }
    }
  }
}

This implies that we would also need a GET /accounts/{id}/sessions/{id} and a DELETE /accounts/{id}/sessions/{id} route, which works for me.

Any objections / thoughts?

current state is at
http://docs.accountjsonapi.apiary.io/#reference/requests/request-collection/create

source is here:
https://github.com/hoodiehq/account-json-api/blob/master/apiary.apib#L725

• start a PR
• Add 201 response: links.self is optional. If no resource is persisted in the database, it won't be set (not relevant anymore, see comment below)
• specify a response if request handler is not implemented (403 maybe)?
• review & merge

We need support a client-generated ID for for Hoodie. JSON API explicitly supports client-generated resource IDs, so let’s put it into the spec, that it’s allowed, but not required of course

To keep the summary on the README inline with the the API docs we should change the the admin route parameters to use id instead of username

Now that I'm implementing the account-json-api spec, I find the /session/account route confusing, because an account can exist and be accessible even without a session. For example, a user can signed up for an account but not sign in until the account is confirmed.

For that reason, I'd suggest to rename /session/account to /account, and /session/account/profile to /account/profile

Update

We now suggest to get rid of /session/account altogether, and use /accounts/{id} instead

According to this:
http://restcookbook.com/HTTP%20Methods/patch/
"Also, the PUT method is idempotent. PUTting the same data multiple times to the same resource, should not result in different resources, while POSTing to the same resource can result in the creation of multiple resources."

Since the response of this request would have the Created resource (the session), which includes a sessionId. and I think multiple requests would end up with different sessionIds. So using PUT is wrong, and it should be a POST.

I liked the idea of using usernames as IDs in the URLs, but it’s inconsistent with all the rest of our APIs, and could potentially cause issue with JSON API clients.

For example GET /accounts/jane-doe would return

{
    "links": {
        "self": "https://example.com/accounts/jane-doe"
    },
    "data": {
        "id": "def678",
        "type": "account",
        "attributes": {
            "username": "jane-doe",
        }
    },
    "relationships": {
        "profile": {
            "links": {
                "related": "https://example.com/accounts/jane-doe/profile"
            },
            "data": {
                "id": "def678-profile",
                "type": "accountProfile"
            }
        }
    }
}

But data.id is "def678", not "jane-doe". I'm not sure if this is JSON API compliant or not, but when in doubt, I'd rather avoid confusion.

Not using the username in the URLs also prevents issues with weird characters in usernames.

• start a pr #11
• make the fix (breaking change)
• review & merge

• requests can defined allowed query arguments
• we could list all models and their relations
• show example for session with ?include=account,profile
• show example for account with ?include=profile

I think we can remove the PATCH /session/account route entirely, and make all account properties after sign up read only, without exception. The security requirements / workflows to change a username or a password are app-specific, so I'd suggest that we use the requests API for that. I think it's perfectly suited for that.

In Hoodie’s implementation, the handlers for the different requests can be defined with requests option passed to the hapi plugin. For example the following request handler would require a x-password header to be sent, and the PUT request against CouchDB would happen with basic auth, using the username and password, and will therefore fail if the password is incorrect. The user's session is ignored entirely.

var options = {
  /// ...
  requests: {
    updateAccount: function (request, reply) {
      var server = request.connection.server
      var user = request.auth.credentials
      var password = request.headers['x-password']

      var promise = server.plugins.account.api.accounts.update(user.id, {
        username: request.payload.username,
        password: request.payload.password
      }, {
        auth: {
          username: user.username,
          password: password
        }
      })

      reply(promise)
    }
  }
}

That would also make the separation of accounts & profiles more clear.

Any thoughts @patriciagarcia @tthew?