-
Notifications
You must be signed in to change notification settings - Fork 55
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
DXCDT-293: Access token management for client credentials #537
Changes from 3 commits
c5027b6
5dd0688
972c191
0b57340
2247df1
6219b13
1a0d1b2
9fa07db
cd306af
5abe26a
4e61126
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import ( | |
"time" | ||
|
||
"github.com/joeshaw/envdecode" | ||
"golang.org/x/oauth2/clientcredentials" | ||
) | ||
|
||
const ( | ||
|
@@ -65,7 +66,7 @@ type Result struct { | |
Domain string | ||
RefreshToken string | ||
AccessToken string | ||
ExpiresIn int64 | ||
ExpiresAt time.Time | ||
} | ||
|
||
type State struct { | ||
|
@@ -167,10 +168,14 @@ func (a *Authenticator) Wait(ctx context.Context, state State) (Result, error) { | |
return Result{}, fmt.Errorf("cannot parse tenant from the given access token: %w", err) | ||
} | ||
|
||
expiresAt := time.Now().Add( | ||
time.Duration(res.ExpiresIn) * time.Second, | ||
) | ||
|
||
return Result{ | ||
RefreshToken: res.RefreshToken, | ||
AccessToken: res.AccessToken, | ||
ExpiresIn: res.ExpiresIn, | ||
ExpiresAt: expiresAt, | ||
Tenant: ten, | ||
Domain: domain, | ||
}, nil | ||
|
@@ -249,3 +254,39 @@ func parseTenant(accessToken string) (tenant, domain string, err error) { | |
} | ||
return "", "", fmt.Errorf("audience not found for %s", audiencePath) | ||
} | ||
|
||
// ClientCredentials encapsulates all data to facilitate access token creation with client credentials (client ID and client secret) | ||
type ClientCredentials struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would've used the |
||
ClientID string | ||
ClientSecret string | ||
Domain string | ||
} | ||
|
||
// GetAccessTokenFromClientCreds generates an access token from client credentials | ||
func GetAccessTokenFromClientCreds(args ClientCredentials) (Result, error) { | ||
u, err := url.Parse("https://" + args.Domain) | ||
if err != nil { | ||
return Result{}, err | ||
} | ||
|
||
credsConfig := &clientcredentials.Config{ | ||
ClientID: args.ClientID, | ||
ClientSecret: args.ClientSecret, | ||
TokenURL: u.String() + "/oauth/token", | ||
EndpointParams: url.Values{ | ||
"client_id": {args.ClientID}, | ||
"scope": {strings.Join(RequiredScopesMin(), " ")}, | ||
"audience": {u.String() + "/api/v2/"}, | ||
}, | ||
} | ||
|
||
resp, err := credsConfig.Token(context.Background()) | ||
if err != nil { | ||
return Result{}, err | ||
} | ||
|
||
return Result{ | ||
AccessToken: resp.AccessToken, | ||
ExpiresAt: resp.Expiry, | ||
}, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,17 +138,10 @@ func (c *cli) setup(ctx context.Context) error { | |
ua = fmt.Sprintf("%v/%v", userAgent, strings.TrimPrefix(buildinfo.Version, "v")) | ||
) | ||
|
||
if t.ClientID != "" && t.ClientSecret != "" { | ||
m, err = management.New(t.Domain, | ||
management.WithClientCredentials(t.ClientID, t.ClientSecret), | ||
management.WithUserAgent(ua), | ||
) | ||
} else { | ||
m, err = management.New(t.Domain, | ||
management.WithStaticToken(t.AccessToken), | ||
management.WithUserAgent(ua), | ||
) | ||
} | ||
m, err = management.New(t.Domain, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one of the major simplifications that occurs when we rely simply on the access token for both authentication methods. |
||
management.WithStaticToken(t.AccessToken), | ||
management.WithUserAgent(ua), | ||
) | ||
|
||
if err != nil { | ||
return err | ||
|
@@ -168,27 +161,47 @@ func (c *cli) prepareTenant(ctx context.Context) (Tenant, error) { | |
return Tenant{}, err | ||
} | ||
|
||
if t.ClientID != "" && t.ClientSecret != "" { | ||
return t, nil | ||
} | ||
|
||
if t.AccessToken == "" || scopesChanged(t) { | ||
t, err = RunLogin(ctx, c, true) | ||
if err != nil { | ||
return Tenant{}, err | ||
} | ||
} else if isExpired(t.ExpiresAt, accessTokenExpThreshold) { | ||
// check if the stored access token is expired: | ||
// use the refresh token to get a new access token: | ||
return t, nil | ||
} | ||
|
||
if isExpired(t.ExpiresAt, accessTokenExpThreshold) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much of this diff is due to the elimination of the |
||
// regenerate access token for client credential auth'd tenants | ||
if t.ClientID != "" && t.ClientSecret != "" { | ||
token, err := auth.GetAccessTokenFromClientCreds(auth.ClientCredentials{ | ||
ClientID: t.ClientID, | ||
ClientSecret: t.ClientSecret, | ||
Domain: t.Domain, | ||
}) | ||
|
||
if err != nil { | ||
return t, err | ||
} | ||
|
||
t := Tenant{ | ||
Domain: t.Domain, | ||
AccessToken: token.AccessToken, | ||
ExpiresAt: token.ExpiresAt, | ||
Scopes: auth.RequiredScopes(), | ||
} | ||
|
||
if err := c.addTenant(t); err != nil { | ||
return t, fmt.Errorf("unexpected error adding tenant to config: %w", err) | ||
} | ||
return t, nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how we "refresh" expired access tokens with client credentials. |
||
|
||
// regenerate access token for device auth'd tenants | ||
tr := &auth.TokenRetriever{ | ||
Authenticator: c.authenticator, | ||
Secrets: &auth.Keyring{}, | ||
Client: http.DefaultClient, | ||
} | ||
|
||
// NOTE(cyx): this code will have to be adapted to instead | ||
// maybe take the clientID/secret as additional params, or | ||
// something similar. | ||
res, err := tr.Refresh(ctx, t.Domain) | ||
if err != nil { | ||
// ask and guide the user through the login process: | ||
|
@@ -197,18 +210,19 @@ func (c *cli) prepareTenant(ctx context.Context) (Tenant, error) { | |
if err != nil { | ||
return Tenant{}, err | ||
} | ||
} else { | ||
// persist the updated tenant with renewed access token | ||
t.AccessToken = res.AccessToken | ||
t.ExpiresAt = time.Now().Add( | ||
time.Duration(res.ExpiresIn) * time.Second, | ||
) | ||
return t, nil | ||
} | ||
|
||
err = c.addTenant(t) | ||
if err != nil { | ||
return Tenant{}, err | ||
} | ||
t.AccessToken = res.AccessToken | ||
t.ExpiresAt = time.Now().Add( | ||
time.Duration(res.ExpiresIn) * time.Second, | ||
) | ||
|
||
err = c.addTenant(t) | ||
if err != nil { | ||
return Tenant{}, err | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the above, this is a stylistic change to eliminate the |
||
} | ||
|
||
return t, nil | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this change enabled me to reuse the
Result
type for client credential access tokens. The associated arithmetic gets slightly shuffled as a result.