nhost / hasura-auth

Authentication for Hasura.
https://nhost.io
MIT License
377 stars 114 forks source link

Handle comma separated allowed roles #391

Closed iangabrielsanchez closed 1 year ago

iangabrielsanchez commented 1 year ago

Fixes https://github.com/nhost/nhost/issues/1488

This fixes the error:

2023-01-09 14:06:59 | hasura-auth | ELIFECYCLE  Command failed with exit code 1.
-- | -- | --
2023-01-09 14:06:59 | hasura-auth | at async /app/dist/routes/oauth/index.js:241:31
2023-01-09 14:06:59 | hasura-auth | at processTicksAndRejections (node:internal/process/task_queues:96:5)
2023-01-09 14:06:59 | hasura-auth | at transformOauthProfile (/app/dist/routes/oauth/utils.js:40:141)
2023-01-09 14:06:59 | hasura-auth | TypeError: ((intermediate value)(intermediate value)(intermediate value) \|\| utils_1.ENV.AUTH_USER_DEFAULT_ALLOWED_ROLES).map is not a function
2023-01-09 14:06:59 | hasura-auth |  
2023-01-09 14:06:59 | hasura-auth | ^
2023-01-09 14:06:59 | hasura-auth | data: ((options === null \|\| options === void 0 ? void 0 : options.allowedRoles) \|\| utils_1.ENV.AUTH_USER_DEFAULT_ALLOWED_ROLES).map((role) => ({
2023-01-09 14:06:59 | hasura-auth | /app/dist/routes/oauth/utils.js:40

When allowedRoles is passed as an option when signing in, I noticed that it sets the session.options from the query then it dereferences it then passes it to the transformOauthProfile which expects a string[], but is actually a comma separated string

Before submitting this PR:

Checklist

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: 7b72b877530225095777f7ee96de7cab1a239ed6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ----------- | ----- | | hasura-auth | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

iangabrielsanchez commented 1 year ago

It looks good. I only have a last comment on the changeset. If you think it's fine, we can merge.

looks good. I have commited your suggestion