Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected app_metadata and user_metadata retrieval. #72

Conversation

typhoon2099
Copy link

app_metadata and user_metadata can't be returned by the userinfo endpoint any more (unless you signed paid for Auth0 before 1st September 2017). This pull request and the following Auth0 rule corrects this:

function (user, context, callback) {
  var namespace = 'http://catalogue/';
   if (context.idToken && user.user_metadata) {
     context.idToken[namespace + 'user_metadata'] = user.user_metadata;
   }
   if (context.idToken && user.app_metadata) {
     context.idToken[namespace + 'app_metadata'] = user.app_metadata;
   }
   callback(null, user, context);
}

@skinkie
Copy link

skinkie commented Jul 10, 2018

Can confirm this works, at least ... the next problem is...
catalogueglobal/datatools-server#102

@abyrd
Copy link

abyrd commented Jul 11, 2018

@landonreed have you tested out this pull request?

@landonreed
Copy link
Member

When attempting to add the rule to our Auth0 account and clicking 'Try this rule', I get: ERROR: Unknown error. Possible reason: you are not calling callback(null, user, context)

@landonreed
Copy link
Member

Ah, never mind. I was including an underscore in the rule description and Auth0 didn't like that.

@landonreed
Copy link
Member

landonreed commented Jul 12, 2018

Adding this rule does not appear to work for our (pre-September 2017) existing Auth0 account; however, it does work for a test account I created a couple of days ago.

In order to accept this PR, the assignment of the profile.app_metadata and profile.user_metadata to the catalogue-scoped values should be wrapped in a conditional, e.g.:

if (typeof profile.user_metadata === 'undefined' && typeof profile.app_metadata === 'undefined') {
  profile.app_metadata = profile['http://catalogue/app_metadata']
...

There should also be sufficient comments about what is going on here (the reason for the assignment and wrapping it in an if block) and a reference to this issue and any other Auth0 documentation about the rule that @typhoon2099 provided above. I would also like to see a check for the existence of the scoped fields within this block and an informative error thrown about the Auth0 rule if it is not found. And an update to the docs regarding Auth0 setup would be greatly appreciated.

As @skinkie mentioned above, this also depends on a server fix, which we'd be happy to review if anyone has some code that's backwards-compatible. Thanks so much for investigating this!

@landonreed
Copy link
Member

landonreed commented Jul 12, 2018

I've also just discovered that adding the above rule to a pre-September 2017 Auth0 account does not play nicely with applications running with the existing Auth setup. So, I'm not entirely sure this rule/hack is a great path forward.

Edit: in other words, when I added the rule to our existing account, authentication seemed to stop working for our applications running the existing code. I'm not sure if there's a way to isolate the application of rules to a single application within an Auth0 tenant.

@landonreed
Copy link
Member

I have just learned that it is possible to check the clientID on the context arg in the rule. For example:

function (user, context, callback) {
  // Only apply rule to certain client
  if (context.clientID === 'ABC123') {
    const namespace = 'http://datatools/';
    if (context.idToken && user.user_metadata) {
      context.idToken[namespace + 'user_metadata'] = user.user_metadata;
    }
    if (context.idToken && user.app_metadata) {
      context.idToken[namespace + 'app_metadata'] = user.app_metadata;
    }
  }
  callback(null, user, context);
}

So for Conveyal's Auth0 tenant, we can apply this rule only to certain applications, which means it won't interfere with our other production applications.

Adding this rule and changing the client to get the user profile from a datatools-server endpoint (rather than using the Auth0 Lock getUserInfo endpoint, should be enough to fix the Auth0 issue.

@landonreed landonreed self-assigned this Oct 10, 2018
@landonreed
Copy link
Member

Closing in favor of #250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants