shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: replace AsSystemRestricted with narrower actors (#23712)

Browse Source 
Replace overly-broad `AsSystemRestricted` with purpose-built actors:

- **OAuth2 provider paths** → `AsSystemOAuth2` (13 call sites across
`tokens.go`, `registration.go`, `apikey.go`)
- **Provisioner daemon health read** → `AsSystemReadProvisionerDaemons`
(1 site in `healthcheck/provisioner.go`)
- **Provisionerd file cache paths** → `AsProvisionerd` (2 sites in
`provisionerdserver.go`, matching existing usage nearby)

<details>
<summary>Implementation notes</summary>

Each replacement actor is a strict subset of `AsSystemRestricted`. Every
DB method
at each call site is already covered by the narrower actor's
permissions:

- `subjectSystemOAuth2`: OAuth2App/Secret/CodeToken (all), ApiKey (Read,
Delete), User (Read), Organization (Read)
- `subjectSystemReadProvisionerDaemons`: ProvisionerDaemon (Read)
- `subjectProvisionerd`: File (Create, Read) plus provisionerd-scoped
resources

No new permissions added. `nolint:gocritic` comments updated to reflect
the new actors.
</details>

> 🤖 Created by a Coder Agent, reviewed by me.
This commit is contained in:
Cian Johnston
2026-03-27 15:08:30 +00:00
committed by GitHub
parent 97a27d3c09
commit 3f55b35f68
5 changed files with 32 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/healthcheck/provisioner.go
+2 -2
View File
@@ -71,8 +71,8 @@ func (r *ProvisionerDaemonsReport) Run(ctx context.Context, opts *ProvisionerDae
return
}
// nolint: gocritic // need an actor to fetch provisioner daemons
daemons, err := opts.Store.GetProvisionerDaemons(dbauthz.AsSystemRestricted(ctx))
// nolint: gocritic // Read-only access to provisioner daemons for health check
daemons, err := opts.Store.GetProvisionerDaemons(dbauthz.AsSystemReadProvisionerDaemons(ctx))
if err != nil {
r.Severity = health.SeverityError
r.Error = ptr.Ref("error fetching provisioner daemons: " + err.Error())
coderd/httpmw/apikey.go
+2 -2
View File
@@ -699,8 +699,8 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
// is being used with the correct audience/resource server (RFC 8707).
func validateOAuth2ProviderAppTokenAudience(ctx context.Context, db database.Store, key database.APIKey, accessURL *url.URL, r *http.Request) error {
// Get the OAuth2 provider app token to check its audience
//nolint:gocritic // System needs to access token for audience validation
token, err := db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemRestricted(ctx), key.ID)
//nolint:gocritic // OAuth2 system context — audience validation for provider app tokens
token, err := db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemOAuth2(ctx), key.ID)
if err != nil {
return xerrors.Errorf("failed to get OAuth2 token: %w", err)
}
coderd/oauth2provider/registration.go
+16 -16
View File
@@ -73,8 +73,8 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi
// Store in database - use system context since this is a public endpoint
now := dbtime.Now()
clientName := req.GenerateClientName()
//nolint:gocritic // Dynamic client registration is a public endpoint, system access required
app, err := db.InsertOAuth2ProviderApp(dbauthz.AsSystemRestricted(ctx), database.InsertOAuth2ProviderAppParams{
//nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint
app, err := db.InsertOAuth2ProviderApp(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppParams{
ID: clientID,
CreatedAt: now,
UpdatedAt: now,
@@ -121,8 +121,8 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi
return
}
//nolint:gocritic // Dynamic client registration is a public endpoint, system access required
_, err = db.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemRestricted(ctx), database.InsertOAuth2ProviderAppSecretParams{
//nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint
_, err = db.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppSecretParams{
ID: uuid.New(),
CreatedAt: now,
SecretPrefix: []byte(parsedSecret.Prefix),
@@ -183,8 +183,8 @@ func GetClientConfiguration(db database.Store) http.HandlerFunc {
}
// Get app by client ID
//nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
writeOAuth2RegistrationError(ctx, rw, http.StatusUnauthorized,
@@ -269,8 +269,8 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger
req = req.ApplyDefaults()
// Get existing app to verify it exists and is dynamically registered
//nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err == nil {
aReq.Old = existingApp
}
@@ -294,8 +294,8 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger
// Update app in database
now := dbtime.Now()
//nolint:gocritic // RFC 7592 endpoints need system access to update dynamically registered clients
updatedApp, err := db.UpdateOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), database.UpdateOAuth2ProviderAppByClientIDParams{
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
updatedApp, err := db.UpdateOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), database.UpdateOAuth2ProviderAppByClientIDParams{
ID: clientID,
UpdatedAt: now,
Name: req.GenerateClientName(),
@@ -377,8 +377,8 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger
}
// Get existing app to verify it exists and is dynamically registered
//nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err == nil {
aReq.Old = existingApp
}
@@ -401,8 +401,8 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger
}
// Delete the client and all associated data (tokens, secrets, etc.)
//nolint:gocritic // RFC 7592 endpoints need system access to delete dynamically registered clients
err = db.DeleteOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
err = db.DeleteOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
"server_error", "Failed to delete client")
@@ -453,8 +453,8 @@ func RequireRegistrationAccessToken(db database.Store) func(http.Handler) http.H
}
// Get the client and verify the registration access token
//nolint:gocritic // RFC 7592 endpoints need system access to validate dynamically registered clients
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
//nolint:gocritic // OAuth2 system context — RFC 7592 registration access token validation
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
// Return 401 for authentication-related issues, not 404
coderd/oauth2provider/tokens.go
+8 -8
View File
@@ -217,8 +217,8 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadSecret
}
//nolint:gocritic // Users cannot read secrets so we must use the system.
dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(secret.Prefix))
//nolint:gocritic // OAuth2 system context — users cannot read secrets
dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(secret.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadSecret
}
@@ -236,8 +236,8 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadCode
}
//nolint:gocritic // There is no user yet so we must use the system.
dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(code.Prefix))
//nolint:gocritic // OAuth2 system context — no authenticated user during token exchange
dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(code.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadCode
}
@@ -384,8 +384,8 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadToken
}
//nolint:gocritic // There is no user yet so we must use the system.
dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(token.Prefix))
//nolint:gocritic // OAuth2 system context — no authenticated user during refresh
dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(token.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadToken
}
@@ -411,8 +411,8 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
}
// Grab the user roles so we can perform the refresh as the user.
//nolint:gocritic // There is no user yet so we must use the system.
prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), dbToken.APIKeyID)
//nolint:gocritic // OAuth2 system context — need to read the previous API key
prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemOAuth2(ctx), dbToken.APIKeyID)
if err != nil {
return codersdk.OAuth2TokenResponse{}, err
}
coderd/provisionerdserver/provisionerdserver.go
+4 -4
View File
@@ -1881,8 +1881,8 @@ func (s *server) completeTemplateImportJob(ctx context.Context, job database.Pro
hashBytes := sha256.Sum256(moduleFiles)
hash := hex.EncodeToString(hashBytes[:])
// nolint:gocritic // Requires reading "system" files
file, err := db.GetFileByHashAndCreator(dbauthz.AsSystemRestricted(ctx), database.GetFileByHashAndCreatorParams{Hash: hash, CreatedBy: uuid.Nil})
//nolint:gocritic // Acting as provisionerd
file, err := db.GetFileByHashAndCreator(dbauthz.AsProvisionerd(ctx), database.GetFileByHashAndCreatorParams{Hash: hash, CreatedBy: uuid.Nil})
switch {
case err == nil:
// This set of modules is already cached, which means we can reuse them
@@ -1893,8 +1893,8 @@ func (s *server) completeTemplateImportJob(ctx context.Context, job database.Pro
case !xerrors.Is(err, sql.ErrNoRows):
return xerrors.Errorf("check for cached modules: %w", err)
default:
// nolint:gocritic // Requires creating a "system" file
file, err = db.InsertFile(dbauthz.AsSystemRestricted(ctx), database.InsertFileParams{
//nolint:gocritic // Acting as provisionerd
file, err = db.InsertFile(dbauthz.AsProvisionerd(ctx), database.InsertFileParams{
ID: uuid.New(),
Hash: hash,
CreatedBy: uuid.Nil,