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

Disable registration when authorization.strategy is password #19

Closed
wants to merge 2 commits into from
Closed

Conversation

nicolai86
Copy link
Contributor

Hey there,

when running squash web I'd like to disable registration since everybody who registers can join an existing project.

This functionality could be integrated fairly easy by adding a new setting to the authorization.yml and some changes to the routes and the sessions/new view, but I wanted to ask first if this feature whould be acccepted upstream.

I'd like to know if you would consider the authorization.yml the correct place to add such a feature. I'd argue that introducing a second authorization level (eg admin, user) and adding a settings page for admins where such things can be changed without having to restart the app would be a good addition.

Any suggestions/ feedback?
I'd be more than happy to provide a pull request later on!

Cheers, Raphael

@RISCfuture
Copy link
Contributor

I'd be opening to accepting it upstream if it's tested and documented. A search for

if Squash::Configuration.authentication.strategy == 'password'

is a good place to start to see my strategy in implementing a lot of this conditional auth strategy code.

@nicolai86
Copy link
Contributor Author

Great. I'll take a look into it and provide a PR so we can discuss this further.

@mlitwiniuk
Copy link
Contributor

@nicolai86 have you started implemented anything? This is crucial for me now ;) So if you haven't started anything, I would love to prepare possibility to turn off registration (via config file for start).

@nicolai86
Copy link
Contributor Author

@mlitwiniuk if already got a running working solution (without tests, tho) but I have not yet had time to make a PR out of it.

@nicolai86
Copy link
Contributor Author

give me a second and I'll provide a PR...

- add Squash::Configuration.authentication.registration_enabled
@nicolai86
Copy link
Contributor Author

@mlitwiniuk there you go, working pull request extracted out of my currently deployed squash instance. Tests will be coming soon, too.

@nicolai86
Copy link
Contributor Author

there you go, simple test. feedback?

@mlitwiniuk
Copy link
Contributor

that was fast, seems to do the trick. Thanks, I'll try that tomorrow.

@RISCfuture
Copy link
Contributor

This looks good initially; I'll be taking a more thorough look this week hopefully.

@RISCfuture
Copy link
Contributor

Committed and pushed with some code tweaks. 05e3836

@RISCfuture RISCfuture closed this Mar 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants