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

Support wildcards in email domain whitelisting #20257

Closed
wants to merge 17 commits into from

Conversation

KingBain
Copy link

@KingBain KingBain commented Jul 5, 2022

Fixes #20255

@KingBain KingBain changed the title Added code to match on asterisk wildcard Support wildcards in email domain whitelisting Jul 5, 2022
services/forms/user_form.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 6, 2022
@Gusted Gusted added the type/enhancement An improvement of existing functionality label Jul 6, 2022
@Gusted Gusted added this to the 1.18.0 milestone Jul 6, 2022
services/forms/user_form.go Outdated Show resolved Hide resolved
@KingBain
Copy link
Author

@lunny @Gusted made the chnages to use a []glob.Glob. I'm hoping this is what you had in mind :)

@Gusted
Copy link
Contributor

Gusted commented Jul 22, 2022

@lunny @Gusted made the chnages to use a []glob.Glob. I'm hoping this is what you had in mind :)

It's going into the right direction. However the best thing you can do here is to only run the compile code once and then store the result.

So please change the code in modules/settings/service.go, such that the []glob.Glob is the stored output on the EmailDomain{Block,White}list. You will need to change the type of those fields([]string -> []glob.Glob) and then simply wrap the current code into the compile glob function(ex. sec.Key("EMAIL_DOMAIN_WHITELIST").Strings(",") -> BuildEmailGlobs(sec.Key("EMAIL_DOMAIN_WHITELIST").Strings(","))) and also move the BuildEmailGlobs function into that file.

@KingBain
Copy link
Author

KingBain commented Sep 12, 2022

Hey, @Gusted thanks for the help. I made the changes. but needed to copy the BuildEmailGlobs function to both service.go and user_form_test.go for everything to work.

was me copying BuildEmailGlobs to user_form_test.go the right thing to do ? Is there a better way to call that function ?

Thanks in advance.

@KingBain
Copy link
Author

KingBain commented Sep 12, 2022

Correction figured out how to call BuildEmailGlobs from setting package in user_form test

@KingBain
Copy link
Author

KingBain commented Sep 12, 2022

I dont want to jinx the process, but if you're happy with the PR. Could we also include a change to rename 'setting.Service.EmailDomainWhitelist' to 'setting.Service.EmailDomainAllowlist' .

I understand why folks stopped using Blacklist, but for symmetry probably worth changing 'Whitelist' to reflect the opposite of 'Blocklist' used in EmailDomainBlocklist .

modules/setting/service.go Outdated Show resolved Hide resolved
modules/setting/service.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

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

Code looks good, might want to add documentation in config-cheat-sheet that EMAIL_DOMAIN_WHITELIST and EMAIL_DOMAIN_BLOCKLIST will be compiled to globs, with a link to https://pkg.go.dev/github.com/gobwas/glob#Compile which list the "syntax".

@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 26, 2022
@lunny lunny modified the milestones: 1.19.0, 1.20.0 Feb 2, 2023
@wxiaoguang
Copy link
Contributor

Is this PR still in progress? Long time since last review suggestion.

At least, the app.example.ini and cheatsheet.md should be updated with "it supports wildcard"

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label May 8, 2023
@wxiaoguang
Copy link
Contributor

Replaced by Support wildcard in email domain allow/block list #24831, since this PR is stale and incomplete.

@wxiaoguang wxiaoguang closed this May 21, 2023
@GiteaBot GiteaBot removed this from the 1.20.0 milestone May 21, 2023
silverwind pushed a commit that referenced this pull request May 22, 2023
Replace #20257 (which is stale and incomplete)

Close #20255

Major changes:

* Deprecate the "WHITELIST", use "ALLOWLIST"
* Add wildcard support for EMAIL_DOMAIN_ALLOWLIST/EMAIL_DOMAIN_BLOCKLIST
* Update example config file and document
* Improve tests
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support wildcards in email domain whitelisting
6 participants