Amplify-js: User Groups Not being updated

Created on 25 Jul 2019  路  25Comments  路  Source: aws-amplify/amplify-js

Describe the bug

Amplify with Cognito. If I update a user's groups, I'm not seeing a great way to update the user's groups. My hope is that this will/should automatically update.

To Reproduce
Steps to reproduce the behavior:

  1. Check user's groups
  async currentUserGroups(): Promise<string[]> {
    const currentSession: CognitoUserSession = await Auth.currentSession();
    let groups: string[] = [];
    if (currentSession) {
      groups = currentSession.getIdToken().payload['cognito:groups'];
    }
    return groups;
  }

Expected behavior

When I update the user's, I would expect Auth.currentSession() to do the right thing.

Sample code
I was able to get the user session to be updated, but this work-around doesn't seem right:

  async updateUserGroups() {
    const currentUser: CognitoUser = await Auth.currentAuthenticatedUser();
    const userSession: CognitoUserSession = currentUser.getSignInUserSession();
    const refreshToken = userSession.getRefreshToken();
    currentUser.refreshSession(refreshToken, (err, session) => {
      currentUser.setSignInUserSession(session);
    });
  }

Or at least it's pretty painful. If this can't happen automatically, is there a way to have a method on CognitoUser ... something like updateUserGroups()?

If this is the "correct" or "best" way to handle this, then at the very least this should be heavily documented. I would be happy to help in any way that I can.

Thank you

Auth Cognito question

Most helpful comment

@haverchuck - this seems like a pretty big security risk to me ... shouldn't this be a marked a bug and escalated?

All 25 comments

@malcomm - Try the bypassCache option in currentAuthenticatedUser:

 Auth.currentAuthenticatedUser({bypassCache: true}).then((res) => console.log('res', res))

@haverchuck - well my code above works. The issue is, I just need the session to be updated automatically if possible. If there was an option for that configuration it would be great. I think adding {bypassCache: true} makes sense, but it still doesn't get around the main issue.

That is ... at least from my perspective.

Just to be sure, the thing I'm trying to do is to limit a user's access to graphql resources. I just realized I did not put this in my original description of the problem ... that's my bad. So what I really meant in the description is that I'm doing @Auth with groupsField and I remove/add a group to a user in Cognito, but their session is not updated and they can still access (or not if I add a group) the graphql resources. For example, if I have group1 (in the groupsField field) on a resource and I have the following:

type Foo
  @model
  @auth(rules: [
    { allow: groups, groupsField: "groupsCanAccess", mutations: [] }
  ])
{
  id: ID!
  ...
}

If a user has group1 at login, they can access that resource. If a Cognito Admin removes group1 from that user and they still have a valid session, they can keep on accessing that resource. If the user does a logout and re-logins, all is good. Or if I do the following:

  async updateUserGroups() {
    const currentUser: CognitoUser = await Auth.currentAuthenticatedUser();
    const userSession: CognitoUserSession = currentUser.getSignInUserSession();
    const refreshToken = userSession.getRefreshToken();
    currentUser.refreshSession(refreshToken, (err, session) => {
      currentUser.setSignInUserSession(session);
    });
  }

Then it fixes this problem. But, that's a client side solution and that seems like a bug.

@malcomm When are you calling updateUserGroups?

@haverchuck - in my case I have an Angular app and I am using an "AuthGuard" via a CanActivate and updateUserGroups() is being called in there.

@haverchuck - just wanted to check on this again ... any updates?

I implemented your code with AppSync, like this:

const client = new AWSAppSyncClient({
  url: AppSyncConfig.aws_appsync_graphqlEndpoint,
  region: AppSyncConfig.aws_project_region,
  auth: {
    type: AppSyncConfig.aws_appsync_authenticationType,
    jwtToken: new Promise(async (res, rej) => {
      const currentUser = await Auth.currentAuthenticatedUser();
      const userSession = currentUser.getSignInUserSession();
      const refreshToken = userSession.getRefreshToken();
      currentUser.refreshSession(refreshToken, (err, session) => {
        if (err) {
          rej(err);
        } else {
          currentUser.setSignInUserSession(session);
          res(session.getIdToken().getJwtToken());
        }
      });
    })
  }
});

Thanks!!

@Muriet96 - thanks for that. But to be sure, isn't that just a different client side solution? My thinking is: this really needs to be a server side fix.

@haverchuck - this seems like a pretty big security risk to me ... shouldn't this be a marked a bug and escalated?

I need this functionality as well. I use Cognito groups to manage user permissions. (unless someone has a better idea on how to manage them).

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@haverchuck - any updates?

If you're using the "responseType":"token" then calling currentAuthenticatedUser will return the un-updated user. you need to either refresh the token yourself or use "responseType":"code"

@oriharel - well the real issue here isn't the groups coming back ... that would be good to fix. The real issue is: when I remove a user from a group, they shouldn't be able to view the data if they don't have permission to do so. I'm using @auth and groupsField; if a user is in group1 they can access that data if group1 is on the field for that table. If I then remove the user from group1, they should no longer be able to view that data. This is not currently the case (it will work if the user logs out or if the token is refreshed, but again, those are client side solutions/workarounds). From what I'm seeing, appsync is using what the client is passing in to verify what groups a user has. This can be modified and is a security risk.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Looks like this got marked stale again ... any updates?

Hi @malcomm,

I'm also looking into this. Thanks for your clear synopsis from above:

If a user has group1 at login, they can access that resource. If a Cognito Admin removes group1 from that user and they still have a valid session, they can keep on accessing that resource. If the user does a logout and re-logins, all is good.
...
But, that's a client side solution and that seems like a bug.

There was a recent CLI update that improves group management (https://aws-amplify.github.io/docs/cli-toolchain/quickstart#group-management and this example: https://aws-amplify.github.io/docs/cli-toolchain/quickstart#administrative-actions), but you're right: the server should be restricting that resource before allowing access, not the client.

@malcomm I've learned that the downstream services are just doing signing validation of the access token, not checking permissions.

We're having conversations on how to best resolve this on both the client & server-side, but unfortunately don't have that answer today.

In the interim, knowing that polling/intervals can be expensive for batter life, I'd recommend the following:

  1. Determine a threshold (e.g. 10 minutes) that you deem reasonable for refreshing tokens.
  2. Leveraging the Page Visibility API, refresh the token when that threshold has lapsed.

Some examples of refreshing tokens have already been provided in this thread, among others:

Thanks for staying on top of this @malcomm! Hopefully the client-side token refresh is a reasonable stopgap while we can research a scalable solution for the server-side.

@ericclemmons - thank you! Glad this is going to be addressed. I was just curious: does Amplify have a bug bounty program? Seems like this might be a good candidate for something like that?

@malcomm just for clarity, there isn't a bug here. JWT tokens contain state from a server which clients pass to backend services. Sessions are not tracked on the backend in such a system and this is a standard industry practice with OIDC tokens where signing validation is completed. What @ericclemmons is suggesting is you could potentially have a trigger on your backend to notify clients when a change has taken place and have them refresh their tokens. This would be an additional feature that would need to be designed.

@ericclemmons & @undefobj - so there's no mechanism in place where the token becomes invalid? Refresh Tokens is what I'm thinking applies here, right?

Also, any solution that requires the client to do something doesn't seem right to me (basically what I'm thinking is that this has to be a server-side fix). If I'm off on this, then please let me know, because I want to learn why my thinking is incorrect here.

@ericclemmons & @undefobj - so there's no mechanism in places where the token becomes invalid? Refresh Tokens is what I'm thinking applies here, right?

No, tokens are valid until they expire. Refresh Tokens are a mechanism for obtaining new IdTokens or AccessTokens without prompting the user to reauthenticate. So you could put something in your backend workflow to trigger a Refresh Token to be used to get new Id/Access Tokens which the client can then use with updated state.

Also, any solution that requires the client to do something doesn't seem right to me (basically what I'm thinking is that this has to be a server-side fix). If I'm off on this, then please let me know, because I want to learn why my thinking is incorrect here.

There are always tradeoffs in systems if you want them to scale. This is an area where there isn't a bulletproof solution. You could store sessions in a central database and have every request go through that, but then you're causing AuthN to be repeated and that server must deserialize and lookup permissions each time which is prone to scale issues, throttling, and even DDoS attacks. This is why claims based AuthZ exists with tokens which expire after a time period.

Essentially what you're looking for is runtime authorization metadata to be dynamically updated. This won't happen without some sort of push based mechanism to clients like I mentioned above. One other alternative however would be to look at seeing if Dynamic Group Authorization with the GraphQL Transformer works for you. Aside from that you'll be hand-rolling runtime logic in a centralized authorization system that needs to scale appropriately.

@undefobj - Thank you for your response. Just to be sure, I am using Dynamic Group Authorization , see my comment above on July 25th.

@ericclemmons & @undefobj - just wanted to loop back on this and see if there are any updates.

Also, I understand there are complexities here that do no make this simple; however, I'm assuming that with Cognito, you must keep track of the tokens on the server side right? Going off of that base assumption I was hoping there would be a "stale" flag where the server would know to initiate an update/refresh. In my mind this is the same if you disable or delete a user (same basic workflow) ... the system surely would have to send back an unauthorized if the user has been disabled via the Cognito UI, right?

And just to be sure: for me, the biggest issue is that server is returning records that the user should not be able to see. I'm not even all that interested in the client token being updated (that would be nice, but not needed at all). What I'm most concerned about is that the GraphQL API is return records that belong to a group via the Dynamic Group Authorization that the user no longer has access to. This might be clear to everyone based on the everything else above, but I just wanted to be sure.

@malcomm I'm not sure there are any updates we could give, it's more of a solution you will need to own. You could try to implement your own solution to dynamically push a notification down to clients and have them refresh their token information, but that's just a suggestion as this integration isn't supported OOTB. As outlined earlier tokens are good until they expire. This is the nature of JWTs and isn't Cognito specific. When using Dynamic Group AuthZ what happens is the "groups that are allowed access" are stored on the data itself on the record in DynamoDB, and the permissions granted for a user are in the JWT which are compared at runtime. So if the JWT has information that hasn't been refreshed yet the client could still have access until the token expires. This is why you would need some sort of push mechanism based on a trigger event in the backend to force the client to refresh its token. This is totally up to what you think your security posture needs to be as most of the time for systems it's fine to let the token expire and refresh in an acceptable time period.

I am going to resolve this issue based on the above comment and information provided early from @undefobj

Was this page helpful?
0 / 5 - 0 ratings

Related issues

romainquellec picture romainquellec  路  3Comments

TheRealRed7 picture TheRealRed7  路  3Comments

karlmosenbacher picture karlmosenbacher  路  3Comments

guanzo picture guanzo  路  3Comments

shinnapatthesix picture shinnapatthesix  路  3Comments