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

clearer access tokens allow check on dotcom #63368

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

sqs
Copy link
Member

@sqs sqs commented Jun 20, 2024

Previously, the code would prevent us from using the AccessTokensAdmin config setting on dotcom entirely, instead of just restricting it when site admins create an access token for a different user, which was the intent.

Test plan

CI

@cla-bot cla-bot bot added the cla-signed label Jun 20, 2024
Comment on lines +54 to +60
// 🚨 SECURITY: Creating access tokens for other users by site admins is not allowed on
// Sourcegraph.com. This check is mostly the defense for a misconfiguration of the site
// configuration.
if dotcom.SourcegraphDotComMode() {
if err := auth.CheckSameUser(ctx, userID); err != nil {
return nil, errors.New("access token creation for other users is disabled on Sourcegraph.com")
}
}

Check notice

Code scanning / Semgrep OSS

Semgrep Finding: security-semgrep-rules.semgrep-rules.generic.comment-tagging-rule Note

Code that highlight SECURITY in comment has changed. Please review the code for changes. The changes might be sensitive.
Previously, the code would prevent us from using the AccessTokensAdmin config setting on dotcom entirely, instead of just restricting it when site admins create an access token for a different user, which was the intent.
@sqs sqs force-pushed the sqs/clearer-dotcom-accesstok-check branch from e8f5de3 to 1573522 Compare June 20, 2024 05:40
@sqs sqs requested a review from a team June 24, 2024 04:24
@sqs sqs merged commit dd32e5f into main Jun 24, 2024
14 checks passed
@sqs sqs deleted the sqs/clearer-dotcom-accesstok-check branch June 24, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants