*: add --enable-automatic-registration flag to worker #463
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericchiang
force-pushed
the
register-on-first-login
branch
from
39ab5cc to
375da03
Compare
|
I think we probably want to carefully think about how we expose this type of knob. In general, this seems like a per-connector configuration, and maybe it's not even something you can configure per-connector, but each connector should probably say whether or not they require registration. Thus:
|
| remoteIdentity := user.RemoteIdentity{ConnectorID: ses.ConnectorID, ID: ses.Identity.ID} | ||
|
|
||
| // Get the connector used to log the user in. | ||
| var conn connector.Connector |
There was a problem hiding this comment.
Breaking this out into its own method, eg .(s *Server) GetConnectorByID(id string) will make this long method a bit easier to read.
|
LGTM after the remaining comments are addressed |
ericchiang
force-pushed
the
register-on-first-login
branch
2 times, most recently
from
ea472ed to
09a6857
Compare
ericchiang
changed the title
|
@sym3tri renamed to "enable-automatic-registration" to match "enable-registration" to show similarity and made them exclusive. |
|
Thanks. LGTM ship it |
| @@ -90,6 +92,11 @@ func main() { | |||
| os.Exit(0) | |||
| } | |||
|
|
|||
| if (*enableRegistration) && (*registerOnFirstLogin) { | |||
| fmt.Fprintln(os.Stderr, "The flags --enable-registration and --register-on-first-login cannot both be true.") | |||
There was a problem hiding this comment.
Typo here register-on-first-login is the old name.
|
scratch that LGTM. found a typo |
For remote connectors, allow users to skip registration.
ericchiang
force-pushed
the
register-on-first-login
branch
from
09a6857 to
35cab93
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When specified the "--register-on-first-login" allows users logging in through remote connectors to skip registration. This make a lot of sense for connectors like LDAP, which have their own registration upstream.
Closes #310
This change also renames "server/testutil.go" to "server/testutil_test.go" to ensure it's not used outside of tests.
cc @sym3tri