DXCDT-293: Access token management for client credentials #537

willvedd · 2022-12-01T20:06:50Z

🔧 Changes

The api command requires an access token to make authenticated management API requests. However, access tokens are not created for tenants authenticated via client credentials, only those authenticated via device code flow. This PR introduces the facilitation of access token management for tenants that are authenticated with client credentials.

Immediately, this work enables the above-mentioned api compatibility, however the intent is broader. This PR retrieves and stores the access token with client credentials. But more importantly, it standardizes both authentication processes and reduces the number of forks in the code to accommodate the two authentication mechanisms. but also lays the groundwork for code simplifications to occur later in time.

🔬 Testing

No tests created directly in this PR, IMO there is insufficient framework to facilitate them. However, the mechanism added will be tested before the running of all integration tests once #532 gets merged in.

📝 Checklist

All new/changed/fixed functionality is covered by tests (or N/A)
I have added documentation for all new/changed functionality (or N/A)

willvedd · 2022-12-01T20:08:21Z

internal/auth/auth.go

@@ -65,7 +66,7 @@ type Result struct {
 	Domain       string
 	RefreshToken string
 	AccessToken  string
-	ExpiresIn    int64
+	ExpiresAt    time.Time


Making this change enabled me to reuse the Result type for client credential access tokens. The associated arithmetic gets slightly shuffled as a result.

willvedd · 2022-12-01T20:09:15Z

internal/auth/auth.go

@@ -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 {


Would've used the clientcredentials.Config type but I wanted to encapsulate the token URL formulation logic within this function.

willvedd · 2022-12-01T20:09:54Z

internal/cli/cli.go

-			management.WithUserAgent(ua),
-		)
-	}
+	m, err = management.New(t.Domain,


This is one of the major simplifications that occurs when we rely simply on the access token for both authentication methods.

willvedd · 2022-12-01T20:10:41Z

internal/cli/cli.go

+		return t, nil
+	}
+
+	if isExpired(t.ExpiresAt, accessTokenExpThreshold) {


Much of this diff is due to the elimination of the else statement in favor of an early return.

willvedd · 2022-12-01T20:11:17Z

internal/cli/cli.go

+		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
+		}


This is how we "refresh" expired access tokens with client credentials.

willvedd · 2022-12-01T20:11:53Z

internal/cli/cli.go

+			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
 		}
+


Similar to the above, this is a stylistic change to eliminate the else in favor of an early return.

sergiught

We reviewed this together with @willvedd and also performed manual tests too on both device code flow and client credentials. We deferred adding some automated unit tests due to the amount of changes needed at the moment to properly mock a couple of components.

DXCDT-293: Access token management for client credentials (#537) * Storing and refreshing access token for client credentials * Removing unnecessary comment * Removing tenant name from being stored, removing flag declarations * Removing tenant name from being stored * Fixing erroneous delete * Simplifying ExpiresAt assignment * Remove duplicate addTenant in tenants add command * Remove setting scopes on tenant when using client credentials * Refactor how we check for token expiration while preparing the tenant * Refactor cli.prepareTenant func * Refactor cli.setup func Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]>

* DXCDT-287: Remove format flag in favor of json flag (#533) * DXCDT-288: Add perms alias for permissions subcommand (#534) * DXCDT-286: Relegate --force flag from global context (#535) * DXCDT-286: Hide global flags from commands when not applicable (#536) * [1/4] DXCDT-266: Move domains subcommand one level up the hierarchy (#539) * [2/4] DXCDT-266: Bring branding emails command under email templates (#540) * Back-merging `main` into `v1` (#543) DXCDT-293: Access token management for client credentials (#537) * Storing and refreshing access token for client credentials * Removing unnecessary comment * Removing tenant name from being stored, removing flag declarations * Removing tenant name from being stored * Fixing erroneous delete * Simplifying ExpiresAt assignment * Remove duplicate addTenant in tenants add command * Remove setting scopes on tenant when using client credentials * Refactor how we check for token expiration while preparing the tenant * Refactor cli.prepareTenant func * Refactor cli.setup func Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> * [3/4] DXCDT-266: Rename branding cmd to universal-login (#541) * [4/4] DXCDT-266: Update docs after branding command refactor (#542) * DXCDT-283: Remove `config` command (#532) Co-authored-by: Will Vedder <[email protected]> * DXCDT-267: Consolidate `auth0 add tenants` into `auth0 login` (1/x) (#546) Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> * DXCDT-267: Graceful handling of access token regeneration (2/x) (#547) Co-authored-by: Rita Zerrizuela <[email protected]> Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> * DXCDT-298: Interactive login prompt (3/x) (#551) Co-authored-by: Rita Zerrizuela <[email protected]> Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> * DXCDT-295: Refactor quickstarts command to use quickstart meta URL (#553) * DXCDT-297: Remove env var ingestion (#554) Removing environment variable ingestion, removing unnecessary comment Co-authored-by: Will Vedder <[email protected]> * DXCDT-271: Add ci step to check that docs are up to date (#560) * DXCDT-271: Move bundle install out of make docs and into docs-start (#562) * DXCDT-296: Supporting additional scopes when authenticating as user (#561) * Adding additional scopes support via --scopes flag * Adding additional scopes support via --scopes flag * Removing logging * Uncommenting scope, removing Start function * Condensing error to single line * Fixing linting errors * Changing test * Updating docs * Unpluralizing text, setting nil default value * Fixing bad help text * Tiny refactors on the login cmd * Fixing linting error * Update internal/auth/auth.go Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Rita Zerrizuela <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> * DXCDT-271: Fix generated docs (#563) * Rename build_doc to doc-gen * Downgrade json flag from persistent to local * Update doc pages * DXCDT-272 Add install script and update README (#564) Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Will Vedder <[email protected]> * DXCDT-273: Authentication documentation (#565) Co-authored-by: Will Vedder <[email protected]> * Updating README * Targeting main branch before we forget to change back Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Will Vedder <[email protected]> Co-authored-by: Sergiu Ghitea <[email protected]> Co-authored-by: Rita Zerrizuela <[email protected]>

willvedd added 3 commits December 1, 2022 14:38

Storing and refreshing access token for client credentials

c5027b6

Removing unnecessary comment

5dd0688

Removing tenant name from being stored, removing flag declarations

972c191

willvedd requested a review from a team as a code owner December 1, 2022 20:06

willvedd commented Dec 1, 2022

View reviewed changes

willvedd added 3 commits December 1, 2022 15:13

Removing tenant name from being stored

0b57340

Fixing erroneous delete

2247df1

Simplifying ExpiresAt assignment

6219b13

willvedd requested review from adamjmcgrath and Widcket December 1, 2022 20:33

sergiught added 5 commits December 2, 2022 15:42

Remove duplicate addTenant in tenants add command

1a0d1b2

Remove setting scopes on tenant when using client credentials

9fa07db

Refactor how we check for token expiration while preparing the tenant

cd306af

Refactor cli.prepareTenant func

5abe26a

Refactor cli.setup func

4e61126

sergiught approved these changes Dec 2, 2022

View reviewed changes

willvedd merged commit 19757dc into main Dec 2, 2022

willvedd deleted the DXCDT-293-store-access-token-client-credentials branch December 2, 2022 16:05

willvedd mentioned this pull request Dec 2, 2022

DXCDT-283: Remove config command #532

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DXCDT-293: Access token management for client credentials #537

DXCDT-293: Access token management for client credentials #537

willvedd commented Dec 1, 2022 •

edited

Loading

willvedd Dec 1, 2022

willvedd Dec 1, 2022

willvedd Dec 1, 2022

willvedd Dec 1, 2022

willvedd Dec 1, 2022

willvedd Dec 1, 2022

sergiught left a comment

DXCDT-293: Access token management for client credentials #537

DXCDT-293: Access token management for client credentials #537

Conversation

willvedd commented Dec 1, 2022 • edited Loading

🔧 Changes

🔬 Testing

📝 Checklist

willvedd Dec 1, 2022

Choose a reason for hiding this comment

willvedd Dec 1, 2022

Choose a reason for hiding this comment

willvedd Dec 1, 2022

Choose a reason for hiding this comment

willvedd Dec 1, 2022

Choose a reason for hiding this comment

willvedd Dec 1, 2022

Choose a reason for hiding this comment

willvedd Dec 1, 2022

Choose a reason for hiding this comment

sergiught left a comment

Choose a reason for hiding this comment

willvedd commented Dec 1, 2022 •

edited

Loading