Feature: Multi-user: Changes to initial admin login processing / implement JWT

[davehorton]

When a fresh system is deployed, there is some first-time login logic that occurs. Currently, upon first logging into a fresh system the admin user is forced to change their password. Once they have set a new password they proceed to the main page of the portal.

Authentication is currently done by means of a pre-generated API key. This is not desirable, and we would like to change to jwt authentication. Therefore the following changes should be made:

jambonz-webapp As before, the first-time user is presented with a change password screen. As before, they will submit the changed password using a PUT /Users/:user_sid request with the api key as the Bearer token. The response, however, will include a jwt, and thereafter the jwt will be used as the Bearer token. Any references to "api key" in the code or the comments should be removed and just referred to as "token" (which I think we are doing in most places anyways)

When logging in after the first time, the /login request will return a jwt, and not an API key.

jambonz-api-server Most of the changes will be on the jambonz-api-server side, to modify the create-password API to return a jwt and to prevent any use of an api key for portal log in with the exception of the first time user. In fact, after allowing a first time user to change their password, the api key used should be recycled.

[kitajchuk]

@davehorton It seems like this one is not quite ready to bite off yet as some backend work needs to be done correct? As for the JWT swap this is easy enough. There are no references to the token as an API key in the code, we simply store it as token and refer to it as such. This is pretty much all handled in the auth context file:

https://github.com/jambonz/jambonz-webapp/blob/main/src/router/auth.tsx

The API index uses the token via the getToken method so we're good there. Swapping from the API key token to the JWT ought to be relatively seamless in this regard.

I think my main question is what data will be encoded in the JWT? We spoke about passing the essential user info and permissions matrix and I still think that would be ideal so we can decode it and slam it into the store without making another API request to a user endpoint. Note that we cannot validate the signature on the client-side but we can decode the data, even without a library. For reference I've pushed an initial branch for this feature and added the JWT parsing function to the auth context file. A draft PR is up for reference:

https://github.com/jambonz/jambonz-webapp/pull/140/files

[EgleHelms]

@kitajchuk @davehorton one thing on the frontend we might need to do is to re-route to the login page again after changing the password for the user to use the new password and the jwt token.

[davehorton]

@EgleHelms I have made the changes to the back end on the the branch named feature/jwt-auth. Can you check out and test against that? This version returns you a token that is a jwt with an expiration of 1 hour (maybe we should make that configurable)?

Currently, you need to test by setting the env JAMBONES_AUTH_USE_JWT to true in order to get a jwt returned as token, so start the integration test like this in the jambonz-api-server folder:

JAMBONES_AUTH_USE_JWT=1 npm run integration-test

The payload you receive now for the POST /login now returns a jwt as the token, and you simply need to use that. Perhaps no changes at all are required on the webapp?

The payload also contains additional information you won't need now, but will be useful as we restrict what the portal shows to service provider or account-level users down the road. Here is an example:

{
    "user_sid": "532c143b-cc09-4a6e-a35a-5308a91ba131",
    "scope": "admin",
    "force_change": true,
    "permissions": ["PROVISION_USERS", "PROVISION_SERVICES", "VIEW_ONLY"],
    "token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX3NpZCI6IjUzMmMxNDNiLWNjMDktNGE2ZS1hMzVhLTUzMDhhOTFiYTEzMSIsInNjb3BlIjoiYWRtaW4iLCJmb3JjZV9jaGFuZ2UiOnRydWUsInBlcm1pc3Npb25zIjpbIlBST1ZJU0lPTl9VU0VSUyIsIlBST1ZJU0lPTl9TRVJWSUNFUyIsIlZJRVdfT05MWSJdLCJpYXQiOjE2NjY3MTUwOTcsImV4cCI6MTY2NjcxODY5N30.acfYZaO5hKXxiN3HCW_-4KcFHgFkqI_RrojN0e_QYUo"
}

Here you are getting the token to use as Bearer token for the session. You are also getting the 'scope', which will be one of:

• admin
• service_provider
• account

Note: if scope=service_provider you will additionally get service_provider_sid, if scope=account you will get account_sid)

And you are getting 'permissions' which will be an array of permissions as shown above. Though we are not doing anything with these at the moment, there are currently 3 permissions that can be granted a user (the admin user above has all three):

• VIEW_ONLY - user can only view service-related data (e.g. calls, alerts, carriers, speech, apps, numbers, etc) but can not change anything, nor can they view or edit users
• PROVISION_SERVICES - user can edit/add/delete all of the service data that is in his or her scope (but can not manage users)
• PROVISION_USERS - user can add/edit/delete users within their scope.

I'm not sure any changes at all are needed on the front end. Things seem to work in the brief testing I did. Unless we want to change the UI to force the user to re-login after setting their first-time password. I'm not sure whether that is needed or not, so feedback from both of you guys would be appreciated.

[davehorton]

@kitajchuk @EgleHelms is there additional information you would like to see in the jwt token? I thought the scope and permissions would be the key thing, but easy enough to return additional info if needed.

[davehorton]

....as a follow up, I suppose if we can parse the jwt then returning {scope, permissions, token} is kind of redundant. I could simply return {token} and you parse the jwt?

I find it odd though that the client can decode the jwt without the encryption secret

[EgleHelms]

@davehorton @kitajchuk Everything works well without any changes, the jwt is given from the initial login with admin/admin creds, so I think that is good enough. Unless we want to force logout and give a new token, since right now the token stays the same since the admin/admin login. Brandon's suggestion works. When I parse the jwt token stored in the localStorage it looks like the object from you return in the payload: { "user_sid": "1c763643-74d9-4a51-aebc-6c38cb16f2f4", "scope": "admin", "force_change": false, "permissions": ["VIEW_ONLY", "PROVISION_SERVICES", "PROVISION_USERS"], "iat": 1666774439, "exp": 1666778039 } But I actually think if we are allowed to store the jwt in the store all the time then we can decode it at any point we need. This will be super useful since we will need to check the role to decide on what to show to the user when we implement the UI changes for different roles. So the role needs to be accessible all the time.

[davehorton]

@kitajchuk when you get a chance can you weigh in on

1. should we merge this PR as is (no forced re-login)
2. should we make the assumption we will always be able to parse the jwt, even in the future, or is this some fluky type of thing we should not depend on?

[kitajchuk]

@kitajchuk when you get a chance can you weigh in on

1. should we merge this PR as is (no forced re-login)
2. should we make the assumption we will always be able to parse the jwt, even in the future, or is this some fluky type of thing we should not depend on?

Let me review things this morning and test it all out. Ideally we parse the JWT and put the user info into the global store. For a client session the app can use the data there (I know we’re not there yet, just saying). Since we use local storage what is the expiry on the JWT? The api key tokens seemed to last forever until you actually logged out and cleared local storage. We may need to test JWT expiration but I think the app would already handle it well with the fetchTransport wrapper. For leaving a client session and returning we need the logic in place to parse the JWT and dispatch the data to the global store.

Again let me catch up later this morning and do a proper review of where we’re at 😅

[davehorton]

jwt expires is configurable but defaults to 1 hour

[kitajchuk]

@davehorton Do we have a refresh token or method or does expiration always mean you get logged out?

[kitajchuk]

....as a follow up, I suppose if we can parse the jwt then returning {scope, permissions, token} is kind of redundant. I could simply return {token} and you parse the jwt?

I find it odd though that the client can decode the jwt without the encryption secret

JWT decoding does not require the secret—signature verification does, which only the backend can do. Technically the data in a JWT is public which is why you wouldn't store anything sensitive in it. This is why you can paste a JWT into jwt.io and get the decoded data without a secret 👍

[kitajchuk]

@davehorton @EgleHelms Okay I think mostly things can work as is with the change on the backend. However I would like to implement the proper updates to our typings etc for API regarding this stuff so lemme do that and I'll push to the branch I have up already.

[kitajchuk]

@davehorton @EgleHelms Okay you can review this PR. I think this is bare minimum to work with the JWT while not actually doing anything with the user data yet. This includes updates to our typings for the API and actually hooking up the user action for the global store which just parses the data from the JWT. Haven't tested expiration yet but I'm assuming it would shake out fine because of how our fetchTransport works.

https://github.com/jambonz/jambonz-webapp/pull/140/files

[davehorton]

@kitajchuk there is no refresh token atm. You just get logged out. Can we accept and merge this PR and do a refresh token as a separate PR if we want one? (I am not up to speed on whether that is the best/common practice, etc)

[davehorton]

PR 140 you mention looks fine for me. Does this mean @EgleHelms should push more changes to this branch now to stick the decoded properties into the global store before we accept and merge this PR?

[kitajchuk]

PR 140 you mention looks fine for me. Does this mean @EgleHelms should push more changes to this branch now to stick the decoded properties into the global store before we accept and merge this PR?

No because I already included that in this PR 👍

[kitajchuk]

@kitajchuk there is no refresh token atm. You just get logged out. Can we accept and merge this PR and do a refresh token as a separate PR if we want one? (I am not up to speed on whether that is the best/common practice, etc)

Yes we should discuss it. Refresh is pretty normal so it might be nice to have that functionality.

[davehorton]

completed