Infrastructure changes for token timeouts #17614
Conversation
openshift-ci-robot
added
the
size/XL
pkg/auth/authenticator/interfaces.go
Outdated
| @@ -13,3 +15,28 @@ type Assertion interface { | |||
| type Client interface { | |||
| AuthenticateClient(client api.Client) (user.Info, bool, error) | |||
| } | |||
|
|
|||
| type OAuthTokenUserValidator interface { | |||
There was a problem hiding this comment.
Should be called OAuthTokenValidator
The fact one of the validators checks users does not change the generic nature of the validator.
| return authenticator.OAuthTokenUserValidatorFunc( | ||
| func(token *oauth.OAuthAccessToken, _ *user.User) error { | ||
| if token.ExpiresIn > 0 { | ||
| if token.CreationTimestamp.Time.Add(time.Duration(token.ExpiresIn) * time.Second).Before(time.Now()) { |
There was a problem hiding this comment.
It be more readbale if the conversion of token.ExpiresIn to a duration would be doen via a helper function
| func TestAuthenticateTokenExpired(t *testing.T) { | ||
| fakeOAuthClient := oauthfake.NewSimpleClientset( | ||
| &oapi.OAuthAccessToken{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}}, | ||
| ObjectMeta: metav1.ObjectMeta{Name: "token1", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}}, // expired |
There was a problem hiding this comment.
these comments are so far they look silly, what about adding just one explanatory comment on the line before &oapi.OAuthAccessToken{ ?
Like:
// Add expired token with 10 minutes expiration time
| tokens oauthclient.OAuthAccessTokenInterface | ||
| users userclient.UserResourceInterface | ||
| groupMapper identitymapper.UserToGroupMapper | ||
| validator authenticator.OAuthTokenUserValidator |
There was a problem hiding this comment.
should probably be plural: validators
pkg/util/sortedset/sortedset.go
Outdated
| @@ -0,0 +1,182 @@ | |||
| package sortedset | |||
There was a problem hiding this comment.
this should be called something like rankedset, a soretedset is a different thing
Also prese move this whole commit to a different PR, as it has nothing to do with the rest of infrastrcture changes
This change removes the duplicate authenticator interfaces that are no longer needed. The exact same interfaces exist upstream, so this change is purely mechanical. Signed-off-by: Monis Khan <[email protected]>
This change adds the OAuthTokenValidator interface for generically validating an OAuthAccessToken and User. The expiration and UID validation was pulled out from the tokenAuthenticator. tokenAuthenticator simply takes OAuthTokenValidators as input, and delegates validation to them. This allows all future validation to simply append itself to the list of validators without requiring any changes to tokenAuthenticator. Signed-off-by: Monis Khan <[email protected]>
This change wires up the OAuth informer to allow performant lookups to be made for the OAuth resources. Signed-off-by: Monis Khan <[email protected]>
This change allows the authenticator to return post start hooks that can be used to set up any infrastructure the authenticator needs. It also makes sure that these resources are properly cleaned up when the post start hooks are stopped. Signed-off-by: Monis Khan <[email protected]>
enj
force-pushed
the
enj/f/token_timeout_foundation
branch
from
804c8d6 to
f27d947
Compare
openshift-ci-robot
added
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, simo5 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest |
|
Automatic merge from submit-queue (batch tested with PRs 17480, 17614). |
Remove duplicate Origin authenticator interfaces
This change removes the duplicate authenticator interfaces that are
no longer needed. The exact same interfaces exist upstream, so this
change is purely mechanical.
Signed-off-by: Monis Khan [email protected]
Add OAuth token and user validator interface
This change adds the OAuthTokenValidator interface for generically
validating an OAuthAccessToken and User. The expiration and UID
validation was pulled out from the tokenAuthenticator.
tokenAuthenticator simply takes OAuthTokenValidators as input, and
delegates validation to them. This allows all future validation to
simply append itself to the list of validators without requiring any
changes to tokenAuthenticator.
Signed-off-by: Monis Khan [email protected]
Wire up OAuth shared informer
This change wires up the OAuth informer to allow performant lookups
to be made for the OAuth resources.
Signed-off-by: Monis Khan [email protected]
Allow authenticator to return post start hooks
This change allows the authenticator to return post start hooks that
can be used to set up any infrastructure the authenticator needs.
It also makes sure that these resources are properly cleaned up when
the post start hooks are stopped.
Signed-off-by: Monis Khan [email protected]
/assign @simo5