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

Add blocklist and allowlist for oidc domains #28

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jawabuu
Copy link

@jawabuu jawabuu commented Apr 23, 2021

This introduces 2 variables
OIDC_DOMAIN_BLOCKLIST
OIDC_DOMAIN_ALLOWLIST
to filter out which email domains are allowed to login when using SSO.
Related issue
#27
It is possible to do away entirely with these new variables if equivalent setting for domains can be found. However, this seems not to be available.
getsentry/self-hosted#2894

@jawabuu
Copy link
Author

jawabuu commented Apr 24, 2021

@max-wittig @ercanucan I have tried as much as possible to keep with your philosophy of keeping the codebase as close as possible to upstream.
The changes made are all opt-in via the 2 env-variables and do not change existing behaviour before the PR.
I also have created a branch from v4.0.1 for compatibility/backporting with the latest release
https://github.com/jawabuu/sentry-auth-oidc/tree/filter-domains-v4.0.1
and this can be merged to existing latest version.

@max-wittig
Copy link
Member

Sorry I totally missed your MR. Didn't get any notifications for it. Will take a look in the next few days!

Copy link
Member

@max-wittig max-wittig left a comment

Choose a reason for hiding this comment

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

Thanks for the MR. Just some suggestions added

oidc/views.py Outdated Show resolved Hide resolved
oidc/views.py Outdated Show resolved Hide resolved
oidc/constants.py Show resolved Hide resolved
@max-wittig
Copy link
Member

Regarding your commit messages. Could you please ensure that these comply with the conventional changelog? Thanks 👍

@jawabuu
Copy link
Author

jawabuu commented May 11, 2021

Thanks @max-wittig I'll address these immediately.

@dlouzan
Copy link
Member

dlouzan commented Feb 26, 2024

@max-wittig Would we want to take another look at this now that builds are fixed again?

/cc @nejch

oidc/views.py Show resolved Hide resolved
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.

3 participants