Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add GetUser to state.CachingAuthClient. #2020

Merged
merged 1 commit into from
Jun 21, 2018
Merged

Add GetUser to state.CachingAuthClient. #2020

merged 1 commit into from
Jun 21, 2018

Conversation

russjones
Copy link
Contributor

@russjones russjones commented Jun 19, 2018

Purpose

During login GetUsers is called and iterated over to find the services.User the user is trying to login as. This makes the login flow sensitive the number of users in the backend. Instead use GetUser get the exact services.User the user is trying to login as.

Implementation

  • Add GetUser to state.CachingAuthClient.
  • Use GetUser in the auth handler.

Related Issues

Fixes #2021

@russjones russjones requested a review from klizhentas June 19, 2018 20:38
@@ -335,18 +335,15 @@ func (h *AuthHandlers) fetchRoleSet(cert *ssh.Certificate, ca services.CertAutho
// for local users, go and check their individual permissions
var roles services.RoleSet
if clusterName == ca.GetClusterName() {
users, err := h.AccessPoint.GetUsers()
// Fetch user from cache.
Copy link
Contributor

Choose a reason for hiding this comment

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

the fact that it's a cache is implementation and configuration dependent, as far as the code is concerned, this is just a client point.

Copy link
Contributor

Choose a reason for hiding this comment

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

(so remove the comment)

@russjones russjones merged commit f91faab into master Jun 21, 2018
@russjones russjones deleted the rjones/get-user branch June 21, 2018 23:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants