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

feat: per-user permissions when using oauth, follow-up #787

Conversation

michael-pattern
Copy link
Contributor

Description

This is a follow-up to #770

After testing, I found that the original implementation had issues that ultimately made the code change unusable. This is a change set to address those issues.

The original change created login entries with null passwords when the OAUTH_PERMISSIONS env var was enabled, but this presents two issues:

  1. If the LOGINS env var is not declared, the app crashes
  2. If the LOGINS env var is declared, OAuth cannot be used

In this PR the implementation is changed, instead of creating users from LOGINS, read all entries of LOGIN_PERMISSIONS_x, and directly create userPermissions entries.

Second issue: hasPermission does not work with OAuth, because the authMiddleware method puts the user information in req.user, and not req.auth. To resolve this issue, I have added a narrow change to hasPermission that allows it to check for req.user.login if and only if OAUTH_PERMISSIONS is enabled.

Testing

If you have a dbgate test environment setup, and an oauth provider you can use, this is roughly how you may setup your environment variables to test this change:

export OAUTH_PERMISSIONS="1" # enables the feature
export OAUTH_CLIENT_ID="your-client-id" # must be filled in per your environment
export OAUTH_CLIENT_SECRET="your-client-secret" # must be filled in per your environment
export OAUTH_AUTH="your-auth-endpoint" # must be filled in per your environment
export OAUTH_TOKEN="your-auth-token-endpoint" # must be filled in per your environment
export OAUTH_SCOPE="your-oauth-scope" # must be filled in per your environment
export OAUTH_LOGIN_FIELD="your-login-field" # must be filled in per your environment, it's probably "sub"

## Permissions
export LOGIN_PERMISSIONS_username="~*, widgets/database" # username should be replaced with the expected username

@michael-pattern
Copy link
Contributor Author

@janproch thank you so much for helping to review and merge my first PR. If you have some time, could I get your assistance with one more PR?

@michael-pattern
Copy link
Contributor Author

Hi @janproch following up here - Could I get your assistance in having this reviewed and merged? Apologies if you are already aware of this PR

@janproch janproch merged commit 4e746a3 into dbgate:master Jun 7, 2024
@janproch
Copy link
Member

janproch commented Jun 7, 2024

@michael-pattern Thanks for this PR, I have merged it.

OAuth authentication is subject of planned bigger changes in next few months, so probably this will be solved finally in different way.

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.

None yet

2 participants