coder · BobbyHo · Aug 10, 2026 · Aug 11, 2026 · Aug 11, 2026 · Aug 11, 2026
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/oauth2.go b/coderd/oauth2.go
@@ -120,7 +120,7 @@ func (api *API) deleteOAuth2ProviderAppSecret() http.HandlerFunc {
 // @Param state query string true "A random unguessable string"
 // @Param response_type query codersdk.OAuth2ProviderResponseType true "Response type"
 // @Param redirect_uri query string false "Redirect here after authorization"
-// @Param scope query string false "Token scopes (currently ignored)"
+// @Param scope query string false "Space-separated scopes to request. Must be within the app's configured scope allowlist; defaults to that allowlist when omitted"
 // @Success 200 "Returns HTML authorization page"
 // @Router /oauth2/authorize [get]
 func (api *API) getOAuth2ProviderAppAuthorize() http.HandlerFunc {
@@ -135,7 +135,7 @@ func (api *API) getOAuth2ProviderAppAuthorize() http.HandlerFunc {
 // @Param state query string true "A random unguessable string"
 // @Param response_type query codersdk.OAuth2ProviderResponseType true "Response type"
 // @Param redirect_uri query string false "Redirect here after authorization"
-// @Param scope query string false "Token scopes (currently ignored)"
+// @Param scope query string false "Space-separated scopes to request. Must be within the app's configured scope allowlist; defaults to that allowlist when omitted"
 // @Success 302 "Returns redirect with authorization code"
 // @Router /oauth2/authorize [post]
 func (api *API) postOAuth2ProviderAppAuthorize() http.HandlerFunc {

diff --git a/coderd/oauth2_metadata_validation_test.go b/coderd/oauth2_metadata_validation_test.go
@@ -541,7 +541,15 @@ func TestOAuth2ClientNameValidation(t *testing.T) {
 	}
 }
 
-// TestOAuth2ClientScopeValidation tests scope parameter validation
+// TestOAuth2ClientScopeValidation tests scope parameter validation at
+// registration time, which accepts any syntactically valid scope string.
+//
+// Registration performs no scope catalog validation, so these values are
+// stored verbatim as the app's scope allowlist. Authorization is where the
+// catalog is enforced: none of the names below is in rbac.IsExternalScope, so
+// an app registered with one can no longer complete an authorization, whether
+// it requests that scope or omits scope entirely. See
+// TestOAuth2AuthorizeDCRScopeCompatibility in coderd/oauth2provider.
 func TestOAuth2ClientScopeValidation(t *testing.T) {
 	t.Parallel()
 
@@ -596,9 +604,11 @@ func TestOAuth2ClientScopeValidation(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name:        "InvalidAdmin",
-			scope:       "admin",
-			expectError: false, // Admin scope should be allowed but validated during authorization
+			name:  "InvalidAdmin",
+			scope: "admin",
+			// Registration accepts it; authorization rejects it with
+			// invalid_scope, since "admin" is not a grantable scope name.
+			expectError: false,
 		},
 		{
 			name:        "ValidCustom",

diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go
@@ -8,6 +8,7 @@ import (
 	htmltemplate "html/template"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"time"
 
@@ -19,10 +20,195 @@ import (
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpapi"
 	"github.com/coder/coder/v2/coderd/httpmw"
+	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/util/slice"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/site"
 )
 
+// Rejection reasons from validateRequestedScope. They are sentinels rather
+// than inline messages so a caller, and the tests, can tell which check
+// failed without matching on message text.
+//
+// Each is wrapped with the offending value ahead of it, because xerrors only
+// wraps without repeating the sentinel's own text when %w is the final verb.
+// These messages are rendered into error_description, so a doubled one is read
+// by a person.
+var (
+	// errUnknownScope is returned for a scope name outside the external scope
+	// catalog, whether unrecognized entirely or recognized but internal-only.
+	errUnknownScope = xerrors.New("unknown or unsupported scope")
+	// errNoGrantableScope is returned when every entry of the app's allowlist
+	// falls outside the catalog, leaving nothing the app can be granted. The
+	// request is not at fault here and may have carried no scope at all, so
+	// the message names the registered list and the only remedy, which is
+	// re-registering the app.
+	errNoGrantableScope = xerrors.New("none of the scopes registered for this app are supported by this deployment; re-register the app with supported scopes")
+	// errScopeNotAllowed is returned for a catalog scope the app's allowlist
+	// does not cover.
+	errScopeNotAllowed = xerrors.New("scope is not in this app's allowed scope list")
+)
+
+// canonicalScopes rewrites each name to the spelling the api_key_scope enum
+// stores and drops repeats, preserving the order of first appearance.
+//
+// It neither validates nor filters: callers check rbac.IsExternalScope
+// separately. Canonicalization is required because rbac.IsExternalScope
+// accepts the aliases `all` and `application_connect`, which are not enum
+// members, so persisting a validated name verbatim can write a value the
+// column's vocabulary does not contain. Deduplicating here keeps the stored
+// value set-valued, which is what a space-separated scope denotes.
+func canonicalScopes(names []string) []string {
+	canonical := make([]string, 0, len(names))
+	for _, name := range names {
+		canonical = append(canonical, string(rbac.CanonicalScopeName(rbac.ScopeName(name))))
+	}
+	return slice.Unique(canonical)
+}
+
+// noScopeAllowlist reports whether an app has no scope allowlist configured.
+// NULL and "" are one state, and this is the only place the two are unified:
+// admin-created apps store sql.NullString{} (apps.go), while DCR-registered
+// apps store Valid: true carrying a possibly-empty req.Scope
+// (registration.go). Once the allowlist decides what a token may do, reading
+// it is an authorization decision, so the two encodings route through one
+// predicate rather than each caller flattening via .String.
+//
+// A whitespace-only allowlist is deliberately not this state. It is a
+// configured value that grants nothing, so it falls through to
+// validateRequestedScope's filtered-to-empty rejection instead of the
+// unrestricted fallback.
+func noScopeAllowlist(appScope sql.NullString) bool {
+	return !appScope.Valid || appScope.String == ""
+}
+
+// validateRequestedScope negotiates the scope the authorization code will
+// carry. Every requested name must be in the external scope catalog (RFC 6749
+// §4.1.2.1 invalid_scope), and the request must be covered by the app's
+// configured allowlist.
+//
+// What each branch returns:
+//
+//	allowlist  request  result
+//	absent     absent   ApiKeyScopeCoderAll, the pre-enforcement grant
+//	absent     present  the request, which is narrower than unrestricted
+//	present    absent   the whole allowlist (RFC 6749 §3.3 default)
+//	present    present  the request, once shown to be within the allowlist
+//
+// An allowlist is absent when NULL or empty, which noScopeAllowlist treats as
+// one state. An allowlist whose every entry falls outside the catalog is
+// rejected rather than read as absent, since falling back there would grant
+// strictly more than the allowlist ever permitted.
+//
+// The return value is written directly to a NOT NULL column whose CHECK
+// constraint also rejects the empty string, so it is a string rather than a
+// []string, and it is never empty alongside a nil error. Its names are
+// canonical api_key_scope spellings and carry no duplicates, so the value can
+// be stored as that enum without further rewriting.
+func validateRequestedScope(requested []string, appScope sql.NullString) (string, error) {
+	// Only names in the external scope catalog (rbac.IsExternalScope) are
+	// user-requestable. That is a curation, not a validity check: RBAC can
+	// expand internal-only names such as debug_info:read just fine, and the
+	// api_key_scope enum would store them, which is exactly why the catalog
+	// exists as a narrower list. Checking here keeps both an unrecognizable
+	// name and an internal-only one out of the granted scope, whether or not
+	// the app has an allowlist to check against.
+	for _, s := range requested {
+		if !rbac.IsExternalScope(rbac.ScopeName(s)) {
+			return "", xerrors.Errorf("%q: %w", s, errUnknownScope)
+		}
+	}
+
+	// Canonicalized after the catalog check, so a rejection names the scope
+	// as the client spelled it rather than as the server stores it.
+	granted := canonicalScopes(requested)
+
+	if noScopeAllowlist(appScope) {
+		if len(requested) == 0 {
+			// Unrestricted, the same grant this app got before scope
+			// enforcement existed, but stated explicitly: an empty string
+			// would violate the column's CHECK.
+			return string(database.ApiKeyScopeCoderAll), nil
+		}
+		return strings.Join(granted, " "), nil
+	}
+
+	// Filter the allowlist through IsExternalScope before it is used for
+	// anything. The allowlist was stored at registration time and may contain
+	// a scope name since removed from the curated catalog, or never in it at
+	// all. Filtering only ever narrows what is granted.
+	allowed := strings.Fields(appScope.String)
+	filtered := make([]string, 0, len(allowed))
+	for _, a := range allowed {
+		if rbac.IsExternalScope(rbac.ScopeName(a)) {
+			filtered = append(filtered, a)
+		}
+	}
+	if len(filtered) == 0 {
+		// The app has an allowlist, but no entry in it is grantable.
+		// Returning the unrestricted sentinel here would grant strictly more
+		// than the allowlist ever permitted, so reject instead. This is the
+		// all-entries-dropped counterpart to the single-stale-entry case the
+		// filter above handles, and it must not share the no-allowlist
+		// branch's fallback.
+		//
+		// Named with the pre-filter list, since that is what was registered
+		// and what the app owner has to change.
+		return "", xerrors.Errorf("%q: %w", strings.Join(allowed, " "), errNoGrantableScope)
+	}
+	// Canonicalized so both sides expand: rbac.ExpandScope knows `coder:all`
+	// and not the `all` alias that IsExternalScope accepts.
+	filtered = canonicalScopes(filtered)
+
+	if len(requested) == 0 {
+		return strings.Join(filtered, " "), nil // RFC 6749 §3.3 default
+	}
+
+	// The allowlist is a ceiling on authority, not a menu of spellings, so the
+	// check is permission coverage rather than name membership. An app allowed
+	// `coder:workspaces.access` can approve a client asking only for
+	// `workspace:read`, which the composite already grants; under name
+	// matching that client's only route to a token was to request the broader
+	// composite instead. Coverage runs against the filtered allowlist, not the
+	// raw one, so a dropped entry grants nothing.
+	allowedNames := make([]rbac.ScopeName, 0, len(filtered))
+	for _, a := range filtered {
+		allowedNames = append(allowedNames, rbac.ScopeName(a))
+	}
+	for _, s := range granted {
+		covered, err := rbac.ScopesCover(allowedNames, rbac.ScopeName(s))
+		if err != nil {
+			// Coverage could not be decided, so the request is refused rather
+			// than granted on an incomplete comparison. %w is last because
+			// xerrors repeats a wrapped message that is not, and this text is
+			// rendered into error_description for a person to read.
+			return "", xerrors.Errorf("%q (%v): %w", s, err, errScopeNotAllowed)
+		}
+		if !covered {
+			return "", xerrors.Errorf("%q: %w", s, errScopeNotAllowed)
+		}
+	}
+	return strings.Join(granted, " "), nil
+}
+
+// consentScopes lists a negotiated scope for the consent page. The
+// unrestricted grant is returned as nil, since "coder:all" states to a user
+// far less than the page's own full-access wording does.
+//
+// The negotiated value is canonical and deduplicated by the time it arrives
+// here, so this splits rather than rewrites.
+func consentScopes(granted string) []string {
+	names := strings.Fields(granted)
+	// Presence, not sole occupancy: an allowlist registered as
+	// `coder:all coder:workspaces.access` defaults to both names, and listing
+	// them would show the user the entry this function exists to avoid showing
+	// while understating a grant that is in fact unrestricted.
+	if slices.Contains(names, string(database.ApiKeyScopeCoderAll)) {
+		return nil
+	}
+	return names
+}
+
 type authorizeParams struct {
 	clientID            string
 	redirectURL         *url.URL
@@ -95,6 +281,37 @@ func extractAuthorizeParams(r *http.Request, callbackURL *url.URL) (authorizePar
 	return params, nil, nil
 }
 
+// redirectAuthorizeError returns an authorization error to the client by
+// redirecting to its callback with the error in the query, which is how
+// RFC 6749 §4.1.2.1 says an authorization request fails once the client is
+// known. Delivering it on Coder instead reaches only the user's screen: the
+// client's error handling never runs, and the state it sent is dropped, so it
+// cannot correlate the failure with the request that caused it.
+//
+// Only errors raised after extractAuthorizeParams returns may use this. Before
+// that point the redirect URI is whatever the request supplied, and §4.1.2.1
+// requires informing the user rather than redirecting to it. Afterwards it has
+// been exact-matched against the app's registered callback, so the destination
+// is the app's own no matter what the request carried.
+func redirectAuthorizeError(rw http.ResponseWriter, r *http.Request, redirectURL *url.URL, state string, code codersdk.OAuth2ErrorCode, description string) {
+	// Copied because the caller's URL is also the consent page's cancel link
+	// and, on the POST side, the success redirect.
+	errorURL := *redirectURL
+	query := errorURL.Query()
+	query.Set("error", string(code))
+	query.Set("error_description", description)
+	// RFC 6749 §4.1.2.1 requires the state back exactly as it arrived,
+	// whenever the client sent one.
+	if state != "" {
+		query.Set("state", state)
+	}
+	errorURL.RawQuery = query.Encode()
+
+	// 302 rather than 307, matching the success redirect below: some external
+	// OAuth2 apps and browsers do not handle 307.
+	http.Redirect(rw, r, errorURL.String(), http.StatusFound)
+}
+
 // ShowAuthorizePage handles GET /oauth2/authorize requests to display the HTML authorization page.
 func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc {
 	return func(rw http.ResponseWriter, r *http.Request) {
@@ -156,6 +373,19 @@ func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc {
 			return
 		}
 
+		// Reject a scope the app can never be granted before the consent page
+		// renders, rather than after the user clicks Allow. Both handlers run
+		// the check for that reason: this one to decide what the page states
+		// and whether it renders at all, the POST side to persist it. The two
+		// negotiate the same query string, since the consent form posts back
+		// to this URL.
+		grantedScope, err := validateRequestedScope(params.scope, app.Scope)
+		if err != nil {
+			redirectAuthorizeError(rw, r, params.redirectURL, params.state,
+				codersdk.OAuth2ErrorCodeInvalidScope, err.Error())
+			return
+		}
+
 		cancel := params.redirectURL
 		cancelQuery := params.redirectURL.Query()
 		cancelQuery.Add("error", "access_denied")
@@ -191,6 +421,7 @@ func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc {
 			DashboardURL: accessURL.String(),
 			CSRFToken:    nosurf.Token(r),
 			Username:     ua.FriendlyName,
+			Scopes:       consentScopes(grantedScope),
 		})
 	}
 }
@@ -234,7 +465,13 @@ func ProcessAuthorize(db database.Store) http.HandlerFunc {
 			return
 		}
 
-		// TODO: Ignoring scope for now, but should look into implementing.
+		grantedScope, err := validateRequestedScope(params.scope, app.Scope)
+		if err != nil {
+			redirectAuthorizeError(rw, r, params.redirectURL, params.state,
+				codersdk.OAuth2ErrorCodeInvalidScope, err.Error())
+			return
+		}
+
 		code, err := GenerateSecret()
 		if err != nil {
 			httpapi.WriteOAuth2Error(r.Context(), rw, http.StatusInternalServerError, codersdk.OAuth2ErrorCodeServerError, "Failed to generate OAuth2 app authorization code")
@@ -271,11 +508,10 @@ func ProcessAuthorize(db database.Store) http.HandlerFunc {
 				CodeChallengeMethod: sql.NullString{String: params.codeChallengeMethod, Valid: params.codeChallengeMethod != ""},
 				StateHash:           hashOAuth2State(params.state),
 				RedirectUri:         sql.NullString{String: params.redirectURL.String(), Valid: params.redirectURIProvided},
-				// Scope negotiation lands in a later phase. Until the
-				// requested scope is validated against the app's allowlist,
-				// persisting it here would store unvalidated client input, so
-				// the code records an unrestricted grant.
-				Scope: string(database.ApiKeyScopeCoderAll),
+				// The negotiated scope, not the requested one: it has been
+				// checked against the scope catalog and the app's allowlist,
+				// and it is what the token minted from this code will carry.
+				Scope: grantedScope,
 			})
 			if err != nil {
 				return xerrors.Errorf("insert oauth2 authorization code: %w", err)