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

api: validate local connector existence before creating user #199

Merged
merged 2 commits into from
Dec 8, 2015

Conversation

ericchiang
Copy link
Contributor

Before creating a user, ensure the local connector ID held by the
overlord and worker match the ID of a local connector in the
database.

Fixes #198

@ericchiang
Copy link
Contributor Author

@bobbyrullo and @joeatwork this one could definitely use a code review.

This commit moves the user.Manage to its own package (user/manager)
so it can import the connector package in a later commit.

For clarity, it renames "Manager" to "UserManager" using gorname.

This commit has no functional changes.
Add ConnectorConfigRepo to UserManager. When trying to create a
RemoteIdentity, validate that the connector ID exists.

Fixes dexidp#198
@ericchiang
Copy link
Contributor Author

@bobbyrullo
First commit has no functional changes. It's just pulling out the user.Manager into its own package.

For the second commit please look at the diff for user/manager/manager.go. All other changes are either fixing tests or compensating for the change in the NewUserManager signature.

@bobbyrullo
Copy link
Contributor

LGTM++

bobbyrullo added a commit that referenced this pull request Dec 8, 2015
api: validate local connector existence before creating user
@bobbyrullo bobbyrullo merged commit 521aeae into dexidp:master Dec 8, 2015
ericchiang added a commit to ericchiang/dex that referenced this pull request Dec 8, 2015
Added missing err return introduced by dexidp#199
bcwaldon added a commit to bcwaldon/dex that referenced this pull request Dec 10, 2015
@ericchiang ericchiang deleted the validate_connector branch December 10, 2015 18:40
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.

2 participants