Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Add scope in before hooks #3023

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Add scope in before hooks #3023

merged 1 commit into from
Sep 16, 2021

Conversation

kimenyikevin
Copy link

No description provided.

@ghost
Copy link

ghost commented Aug 30, 2021

Congratulations 🎉. DeepCode analyzed your code in 3.778 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

Copy link
Member

@barankyle barankyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'Scene' admin page selector in the sidebar needs to be not rendered if the user doesn't have the 'scene:read' scope. The top-level 'Scene' accordion should not be rendered if none of its sub-selectors are selectable.

The top-level 'Location' accordion selector also needs to be turned off if none of its child selectors are rendered.

The various admin pages need to not be hittable without the corresponding read scope. Right now I can go to /admin/groups and see groups even without 'group:read'; not having it be selectable from the sidebar is not enough.

All /editor pages need to not be hittable if the user doesn't have 'editor:write' scope.

contentPacks, group, instance, invite, and party need write scopes as well as read scopes, and those write scopes need to be protecting the create/update/patch/remove functions on their respective services

@barankyle
Copy link
Member

barankyle commented Sep 8, 2021

We want to have default scopes that are granted to users based on their userRole. This will require some minor changes to user creation/patching.

There will be a couple of new environment variables in .env.local.default, DEFAULT_GUEST_SCOPES and DEFAULT_USER_SCOPES. They'll be comma-separated strings, e.g. user:read,user:write,editor:write.

appconfig.ts will have a new sub-field on its export, scopes, that will specify which scopes certain userRoles will get when they're created/patched. They should be sourced from the respective environment variable and split on the commas to produce an array, or default to an empty array if the environment variable is null or an empty string. Here's an example of what it should look like, with guest roughly how it'll be pulled from the ENV_VAR and user an example of what the array should look like:

scopes: {
    guest: DEFAULT_GUEST_SCOPES?.split(',') || [],
    user: [
        'editor:write',
        'location:read',
        'location:write'
    ]
}

When a new user is created in identity-provider.class.ts:create(), a new scope for all of the scopes in config.scopes.guest should be made for that user. user.class.ts:patch needs to be added and, if their userRole is being patched to user, all of the scopes in config.scopes.user should be made for them. If they're being patched to an admin, give them every scope that's in the scopeType table. If they're being downgraded from an admin to a user, remove all of the scopes they have except the ones in config.scopes.user.

DEFAULT_GUEST_SCOPES in .env.local.default should be editor:write,location:read,location:write.
DEFAULT_USER_SCOPES can be the same.
We mainly want local devs to be able to make scenes and locations without having to log in, since that would require also giving out OAuth keys.

@kimenyikevin kimenyikevin force-pushed the ft-add-scope-to-hooks branch 2 times, most recently from ad77e76 to 8763a77 Compare September 9, 2021 08:39
@barankyle
Copy link
Member

I can't see or select the location list from /editor/projects.

Need to add scopes globalAvatars:write, groups:write, instance:write, party:write, user:write, and contentPacks:write.

@barankyle barankyle force-pushed the ft-add-scope-to-hooks branch from 32d60fe to 9a22759 Compare September 16, 2021 22:22
@barankyle barankyle merged commit eaf8b5b into dev Sep 16, 2021
@barankyle barankyle deleted the ft-add-scope-to-hooks branch September 16, 2021 22:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants