Skip to content
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-597: Don't require all scopes for client credentials auth #917

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Nov 20, 2023

🔧 Changes

As noted by #556, when authenticating to the CLI with client credentials, it is necessary for that client to have a set of required scopes. Doing so ensures that the CLI has full functionality out-the-box, however, this precludes the CLI from adhering to principle of least privilege. As the original Github issue notes, the CLI can be used in automation for specific cases and users should be able to provision only the scopes for those cases.

📚 References

Related Github issue: #556

🔬 Testing

Manual verification; no testing framework in place to test token exchange.

📝 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 willvedd requested a review from a team as a code owner November 20, 2023 16:27
@@ -116,6 +116,29 @@ func WaitUntilUserLogsIn(ctx context.Context, httpClient *http.Client, state Sta
}
}

var RequiredScopes = []string{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, doesn't make sense to have these scopes as a standalone file. Decided to pull-in to the auth.go file.

@willvedd
Copy link
Contributor Author

Confirmed again that this works as expected; no other logic needs to be modified.

@willvedd willvedd merged commit de12895 into main Nov 21, 2023
8 checks passed
@willvedd willvedd deleted the DXCDT-597-dont-require-all-scopes-client-creds branch November 21, 2023 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants