Skip to content

Add login hooks.#24828

Merged
mdwn merged 8 commits intomasterfrom
mike.wilson/login-hooks
Apr 24, 2023
Merged

Add login hooks.#24828
mdwn merged 8 commits intomasterfrom
mike.wilson/login-hooks

Conversation

@mdwn
Copy link
Copy Markdown
Contributor

@mdwn mdwn commented Apr 19, 2023

Login hooks have been added to support performing arbitrary operations on user login. This is done to support generating of an Okta assignment on user login for the Okta service feature.

Please refer to https://github.com/gravitational/teleport.e/blob/master/rfd/0003e-application-access-okta-integration.md#teleport-to-okta-user-reconciliation for more information.

Note: This diverges slightly from the RFD as it only happens on login as opposed to happening regularly.

There will need to be a follow on PR to handle SAML/OIDC logins.

Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

Do we need to add CallLoginHooks for SAML/OIDC connectors too? This seems like something that will be very easy to forget to add if any new login processing is added.

Comment thread lib/auth/auth.go Outdated
@mdwn
Copy link
Copy Markdown
Contributor Author

mdwn commented Apr 20, 2023

Do we need to add CallLoginHooks for SAML/OIDC connectors too? This seems like something that will be very easy to forget to add if any new login processing is added.

It does, but that's a PR for e. I'll post it now just so it's associated. Edit: Just to respond to very easy to forget, that's true. I don't know that there's much of a way around this, though -- the same goes for LoginRules.

Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Apologies for the delay!

Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/methods.go Outdated
Comment thread lib/auth/methods.go Outdated
Mike Wilson and others added 6 commits April 21, 2023 10:07
Login hooks have been added to support performing arbitrary operations on
user login. This is done to support generating of an Okta assignment on
user login for the Okta service feature.
Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@mdwn mdwn force-pushed the mike.wilson/login-hooks branch from d77be1f to 3f4c723 Compare April 21, 2023 14:17
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

How are you going to actually use these login hooks? Presumably, you'll need to reconcile user's Okta assignments based on the access requests. The login hook is right now just a function with context and a user - is this enough information in the hook to determine whether you need to create assignments or not? And how would it create them?

TBH I think it would make sense to pair this change with the actual functionality you're introducing it for. Then we can better decide whether this interface works or needs more tweaks.

Also, do the hooks get called for logins via CLI as well?

@mdwn
Copy link
Copy Markdown
Contributor Author

mdwn commented Apr 21, 2023

How are you going to actually use these login hooks? Presumably, you'll need to reconcile user's Okta assignments based on the access requests. The login hook is right only just a function with context and a user - is this enough information in the hook to determine whether you need to create assignments or not?

TBH I think it would make sense to pair this change with the actual functionality you're introducing it for. Then we can better decide whether this interface works or needs more tweaks.

Also, do the hooks get called for logins via CLI as well?

They should get called via the CLI. Basically enterprise will do a

register.LoginHook(oktaUserAssignmentCreator)

And the creator will take a:

	accessInfo := services.AccessInfoFromUser(user)
	checker, err := services.NewAccessChecker(accessInfo, clusterName.GetClusterName(), s)
	if err != nil {
		return nil, trace.Wrap(err)
	}

Then we'll use that checker against the groups and apps, which should give us enough of what we need.

@mdwn
Copy link
Copy Markdown
Contributor Author

mdwn commented Apr 24, 2023

https://github.com/gravitational/teleport.e/pull/1198 Here's the associated e PR. Let me know what you think.

Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/methods.go
@codingllama
Copy link
Copy Markdown
Contributor

LGTM, assuming Roman is happy with the replies.

Comment thread lib/auth/auth.go
@mdwn mdwn enabled auto-merge April 24, 2023 16:21
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from camscale April 24, 2023 16:22
@mdwn mdwn added this pull request to the merge queue Apr 24, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Apr 24, 2023
@mdwn mdwn added this pull request to the merge queue Apr 24, 2023
Merged via the queue into master with commit 5d6b5ad Apr 24, 2023
@mdwn mdwn deleted the mike.wilson/login-hooks branch April 24, 2023 18:24
mdwn added a commit that referenced this pull request Apr 24, 2023
* Add login hooks.

Login hooks have been added to support performing arbitrary operations on
user login. This is done to support generating of an Okta assignment on
user login for the Okta service feature.

* Don't use error channel for calling hooks, test login hooks.

* Expose ResetLoginHooks for external testing.

* Provide user as part of login hook.

* Update lib/auth/methods.go

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Improve the documentation for LoginHook, AuthenticateUser returns types.User.

* Use user.GetName() instead of username in AuthenticateSSHUser response.

* Address nits and restore comments.

---------

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@mdwn mdwn mentioned this pull request Apr 24, 2023
mdwn added a commit that referenced this pull request Apr 24, 2023
* Add login hooks.

Login hooks have been added to support performing arbitrary operations on
user login. This is done to support generating of an Okta assignment on
user login for the Okta service feature.

* Don't use error channel for calling hooks, test login hooks.

* Expose ResetLoginHooks for external testing.

* Provide user as part of login hook.

* Update lib/auth/methods.go

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Improve the documentation for LoginHook, AuthenticateUser returns types.User.

* Use user.GetName() instead of username in AuthenticateSSHUser response.

* Address nits and restore comments.

---------

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@mdwn mdwn mentioned this pull request Apr 24, 2023
mdwn added a commit that referenced this pull request Apr 24, 2023
* Add login hooks.

Login hooks have been added to support performing arbitrary operations on
user login. This is done to support generating of an Okta assignment on
user login for the Okta service feature.

* Don't use error channel for calling hooks, test login hooks.

* Expose ResetLoginHooks for external testing.

* Provide user as part of login hook.

* Update lib/auth/methods.go



* Improve the documentation for LoginHook, AuthenticateUser returns types.User.

* Use user.GetName() instead of username in AuthenticateSSHUser response.

* Address nits and restore comments.

---------

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
mdwn added a commit that referenced this pull request Apr 25, 2023
* Add login hooks.

Login hooks have been added to support performing arbitrary operations on
user login. This is done to support generating of an Okta assignment on
user login for the Okta service feature.

* Don't use error channel for calling hooks, test login hooks.

* Expose ResetLoginHooks for external testing.

* Provide user as part of login hook.

* Update lib/auth/methods.go



* Improve the documentation for LoginHook, AuthenticateUser returns types.User.

* Use user.GetName() instead of username in AuthenticateSSHUser response.

* Address nits and restore comments.

---------

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@r0mant r0mant mentioned this pull request Jul 14, 2023
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.

4 participants