mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
feat: warn before narrowing a self-registered OAuth2 app's scope allowlist (#29777)
- Warn rather than block when admins narrow a self-registered OAuth2 app's scope allowlist. Clients requesting all advertised scopes may receive `invalid_scope` on new authorizations; existing tokens retain their scopes. - Expose `dynamically_registered` in API responses and display registration type on the app page. - Show no warning for widening or clearing allowlists, or for admin-created apps. - Update scope and `invalid_scope` troubleshooting docs and remove the outdated note that the UI cannot set allowlists. Closes PLAT-664.
This commit is contained in:
Generated
+4
@@ -25639,6 +25639,10 @@ const docTemplate = `{
|
||||
}
|
||||
]
|
||||
},
|
||||
"dynamically_registered": {
|
||||
"description": "DynamicallyRegistered is true when the app registered itself through\nDynamic Client Registration rather than being created by an admin.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"endpoints": {
|
||||
"description": "Endpoints are included in the app response for easier discovery. The OAuth2\nspec does not have a defined place to find these (for comparison, OIDC has\na '/.well-known/openid-configuration' endpoint).",
|
||||
"allOf": [
|
||||
|
||||
Generated
+4
@@ -23451,6 +23451,10 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"dynamically_registered": {
|
||||
"description": "DynamicallyRegistered is true when the app registered itself through\nDynamic Client Registration rather than being created by an admin.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"endpoints": {
|
||||
"description": "Endpoints are included in the app response for easier discovery. The OAuth2\nspec does not have a defined place to find these (for comparison, OIDC has\na '/.well-known/openid-configuration' endpoint).",
|
||||
"allOf": [
|
||||
|
||||
@@ -487,6 +487,10 @@ func OAuth2ProviderApp(accessURL *url.URL, dbApp database.OAuth2ProviderApp) cod
|
||||
Icon: dbApp.Icon,
|
||||
Scope: rbac.CanonicalScopeList(dbApp.Scope.String),
|
||||
ClientType: codersdk.OAuth2ClientType(dbApp.ClientType),
|
||||
// The column allows NULL, although every write path sets a value. A
|
||||
// NULL reads as false, which keeps the client update guards closed but
|
||||
// hides the scope narrowing warning for that app.
|
||||
DynamicallyRegistered: dbApp.DynamicallyRegistered.Bool,
|
||||
Endpoints: codersdk.OAuth2AppEndpoints{
|
||||
Authorization: accessURL.ResolveReference(&url.URL{
|
||||
Path: "/oauth2/authorize",
|
||||
|
||||
@@ -284,6 +284,7 @@ func TestOAuth2ProviderAppValidation(t *testing.T) {
|
||||
require.Equal(t, test.callbackURL, updated.CallbackURL)
|
||||
require.Equal(t, []string{test.callbackURL}, updated.RedirectURIs)
|
||||
require.Equal(t, codersdk.OAuth2ClientTypePublic, updated.ClientType)
|
||||
require.True(t, updated.DynamicallyRegistered)
|
||||
|
||||
// The edit replaces the registered URI rather than adding to it.
|
||||
stored, err := db.GetOAuth2ProviderAppByID(ctx, appID)
|
||||
@@ -706,6 +707,33 @@ func TestOAuth2ProviderAppOperations(t *testing.T) {
|
||||
require.Equal(t, "coder:all workspace:read", admin.Scope)
|
||||
require.Equal(t, admin.Scope, dcr.Scope)
|
||||
})
|
||||
|
||||
t.Run("RegistrationOrigin", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
//nolint:gocritic // OAuth2 app management requires owner permission.
|
||||
admin, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
|
||||
Name: testutil.GetRandomName(t),
|
||||
CallbackURL: "https://example.com/callback",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.False(t, admin.DynamicallyRegistered)
|
||||
|
||||
registered, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
|
||||
ClientName: testutil.GetRandomName(t),
|
||||
RedirectURIs: []string{"https://example.com/callback"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
//nolint:gocritic // OAuth2 app management requires owner permission.
|
||||
dcr, err := client.OAuth2ProviderApp(ctx, uuid.MustParse(registered.ClientID))
|
||||
require.NoError(t, err)
|
||||
require.True(t, dcr.DynamicallyRegistered)
|
||||
})
|
||||
}
|
||||
|
||||
func requireCallbackURLValidationError(t *testing.T, err error) {
|
||||
|
||||
@@ -30,6 +30,9 @@ type OAuth2ProviderApp struct {
|
||||
|
||||
// ClientType is "confidential" or "public".
|
||||
ClientType OAuth2ClientType `json:"client_type"`
|
||||
// DynamicallyRegistered is true when the app registered itself through
|
||||
// Dynamic Client Registration rather than being created by an admin.
|
||||
DynamicallyRegistered bool `json:"dynamically_registered"`
|
||||
|
||||
// Endpoints are included in the app response for easier discovery. The OAuth2
|
||||
// spec does not have a defined place to find these (for comparison, OIDC has
|
||||
|
||||
@@ -63,6 +63,7 @@ Treat the setting as a way to stop new authorizations rather than as a way to re
|
||||
- **Name**: Your application name
|
||||
- **Callback URL**: `https://yourapp.example.com/callback` (web) or `myapp://callback` (native/desktop)
|
||||
- **Icon**: Optional icon URL
|
||||
- **Allowed scopes**: Optional. Refer to [Scopes](#scopes).
|
||||
|
||||
### Method 2: Management API
|
||||
|
||||
@@ -341,10 +342,19 @@ The client may then request anything that allowlist covers, and is granted the w
|
||||
An application with no allowlist honors any requested scope, and a request that names no scope is granted `coder:all`.
|
||||
|
||||
An application registered through [Dynamic Client Registration](#dynamic-client-registration) declares its allowlist in the `scope` field of its registration.
|
||||
An administrator sets one with the optional, space-separated `scope` field when [creating an application](../../reference/api/enterprise.md#create-oauth2-application) through the management API.
|
||||
An administrator sets one with the **Allowed scopes** field in the web UI, or with the optional, space-separated `scope` field when [creating an application](../../reference/api/enterprise.md#create-oauth2-application) through the management API.
|
||||
When [updating an application](../../reference/api/enterprise.md#update-oauth2-application), omit `scope` to keep the current allowlist, send a new value to replace it, or send an empty string to clear it and make the application unrestricted.
|
||||
The stored value is not checked against the scopes this deployment offers; a name it does not offer fails at authorization, as described under ["invalid_scope" returned to your callback](#invalid_scope-returned-to-your-callback).
|
||||
The web UI does not yet set the allowlist.
|
||||
|
||||
Use caution when narrowing the scope allowlist of a self-registered application.
|
||||
Many clients request every scope Coder advertises in `scopes_supported` rather than selecting specific scopes; MCP clients that rely on discovery commonly work this way.
|
||||
The allowlist is stored on the application and does not affect that advertised list, so the client continues requesting the full set.
|
||||
Coder rejects requests that exceed the allowlist rather than trimming their scopes, so every new authorization attempt fails with `invalid_scope`.
|
||||
Previously issued tokens retain their scopes.
|
||||
Such a client cannot be restricted through its allowlist: narrowing it breaks the client, and clearing it leaves the application unrestricted.
|
||||
Only a client that can be configured to request fewer scopes can be narrowed, and whoever operates that client makes the change.
|
||||
An allowlist set by an administrator also does not hold against the client: the holder of the application's `registration_access_token` can replace or clear it at any time with `PUT /oauth2/clients/{client_id}`.
|
||||
The web UI warns before saving a narrower allowlist, and the application page indicates whether the application was self-registered or created by an administrator.
|
||||
|
||||
The consent page states the scope being granted before the user approves it. A refresh keeps the scope originally granted; a refresh that names a narrower `scope` applies it to the access token it mints, leaving the grant itself unchanged.
|
||||
|
||||
@@ -517,6 +527,7 @@ opens with the requested name that caused the rejection:
|
||||
`GET /.well-known/oauth-authorization-server`.
|
||||
- `scope requests permissions beyond this app's allowed scopes`: the name is supported, but the application's `scope` allowlist does not cover it.
|
||||
Request less, or widen the allowlist.
|
||||
If the application registered itself and an administrator has since narrowed its allowlist, refer to [Scopes](#scopes).
|
||||
- `none of the scopes registered for this app are supported by this deployment`: the application's `scope` allowlist names nothing this deployment offers, so no request against it can succeed, including one that omits `scope`.
|
||||
Update the allowlist with supported scopes.
|
||||
This description stands alone.
|
||||
@@ -799,7 +810,6 @@ These rules apply to every entry in `redirect_uris`, not only the first one.
|
||||
|
||||
The current implementation has these limitations:
|
||||
|
||||
- The web UI cannot set or change a scope allowlist; declare one at [Dynamic Client Registration](#dynamic-client-registration) or set it through the management API, as described under [Scopes](#scopes)
|
||||
- No client credentials grant support
|
||||
- No device authorization grant support (RFC 8628)
|
||||
- Implicit grant (`response_type=token`) is not supported; OAuth 2.1 deprecated this flow due to token leakage risks, and a request for it redirects to the registered callback with `unsupported_response_type`
|
||||
@@ -849,7 +859,7 @@ For the full error details, refer to ["invalid_scope" returned to your callback]
|
||||
To fix an affected application, the party that holds its `registration_access_token` updates the registration with `PUT /oauth2/clients/{client_id}`, so that `scope` lists only names from `scopes_supported` in `GET /.well-known/oauth-authorization-server`.
|
||||
If that token is lost, register the application again.
|
||||
A Coder administrator can also fix it from the management API by [updating the application](../../reference/api/enterprise.md#update-oauth2-application) with a `scope` that lists supported names, or with an empty `scope` to remove the allowlist.
|
||||
The web UI cannot change it.
|
||||
The **Allowed scopes** field on the application page in the web UI makes the same change.
|
||||
|
||||
## Standards Compliance
|
||||
|
||||
|
||||
Generated
+20
-15
@@ -1519,6 +1519,7 @@ curl -X GET http://coder-server:8080/api/v2/oauth2-provider/apps \
|
||||
{
|
||||
"callback_url": "string",
|
||||
"client_type": "confidential",
|
||||
"dynamically_registered": true,
|
||||
"endpoints": {
|
||||
"authorization": "string",
|
||||
"device_authorization": "string",
|
||||
@@ -1546,21 +1547,22 @@ curl -X GET http://coder-server:8080/api/v2/oauth2-provider/apps \
|
||||
|
||||
Status Code **200**
|
||||
|
||||
| Name | Type | Required | Restrictions | Description |
|
||||
|---------------------------|----------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| `[array item]` | array | false | | |
|
||||
| `» callback_url` | string | false | | Deprecated: equal to the first entry of redirect_uris. Read redirect_uris instead. |
|
||||
| `» client_type` | [codersdk.OAuth2ClientType](schemas.md#codersdkoauth2clienttype) | false | | Client type is "confidential" or "public". |
|
||||
| `» endpoints` | [codersdk.OAuth2AppEndpoints](schemas.md#codersdkoauth2appendpoints) | false | | Endpoints are included in the app response for easier discovery. The OAuth2 spec does not have a defined place to find these (for comparison, OIDC has a '/.well-known/openid-configuration' endpoint). |
|
||||
| `»» authorization` | string | false | | |
|
||||
| `»» device_authorization` | string | false | | Device authorization is optional. |
|
||||
| `»» token` | string | false | | |
|
||||
| `»» token_revoke` | string | false | | |
|
||||
| `» icon` | string | false | | |
|
||||
| `» id` | string(uuid) | false | | |
|
||||
| `» name` | string | false | | |
|
||||
| `» redirect_uris` | array | false | | Redirect uris are the app's registered redirect URIs, primary first. |
|
||||
| `» scope` | string | false | | Scope is the space-separated list of scopes this app's tokens may be granted. Empty means unrestricted. A non-empty value with no names is a configured allowlist that grants nothing. |
|
||||
| Name | Type | Required | Restrictions | Description |
|
||||
|----------------------------|----------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| `[array item]` | array | false | | |
|
||||
| `» callback_url` | string | false | | Deprecated: equal to the first entry of redirect_uris. Read redirect_uris instead. |
|
||||
| `» client_type` | [codersdk.OAuth2ClientType](schemas.md#codersdkoauth2clienttype) | false | | Client type is "confidential" or "public". |
|
||||
| `» dynamically_registered` | boolean | false | | Dynamically registered is true when the app registered itself through Dynamic Client Registration rather than being created by an admin. |
|
||||
| `» endpoints` | [codersdk.OAuth2AppEndpoints](schemas.md#codersdkoauth2appendpoints) | false | | Endpoints are included in the app response for easier discovery. The OAuth2 spec does not have a defined place to find these (for comparison, OIDC has a '/.well-known/openid-configuration' endpoint). |
|
||||
| `»» authorization` | string | false | | |
|
||||
| `»» device_authorization` | string | false | | Device authorization is optional. |
|
||||
| `»» token` | string | false | | |
|
||||
| `»» token_revoke` | string | false | | |
|
||||
| `» icon` | string | false | | |
|
||||
| `» id` | string(uuid) | false | | |
|
||||
| `» name` | string | false | | |
|
||||
| `» redirect_uris` | array | false | | Redirect uris are the app's registered redirect URIs, primary first. |
|
||||
| `» scope` | string | false | | Scope is the space-separated list of scopes this app's tokens may be granted. Empty means unrestricted. A non-empty value with no names is a configured allowlist that grants nothing. |
|
||||
|
||||
#### Enumerated Values
|
||||
|
||||
@@ -1612,6 +1614,7 @@ curl -X POST http://coder-server:8080/api/v2/oauth2-provider/apps \
|
||||
{
|
||||
"callback_url": "string",
|
||||
"client_type": "confidential",
|
||||
"dynamically_registered": true,
|
||||
"endpoints": {
|
||||
"authorization": "string",
|
||||
"device_authorization": "string",
|
||||
@@ -1663,6 +1666,7 @@ curl -X GET http://coder-server:8080/api/v2/oauth2-provider/apps/{app} \
|
||||
{
|
||||
"callback_url": "string",
|
||||
"client_type": "confidential",
|
||||
"dynamically_registered": true,
|
||||
"endpoints": {
|
||||
"authorization": "string",
|
||||
"device_authorization": "string",
|
||||
@@ -1730,6 +1734,7 @@ curl -X PUT http://coder-server:8080/api/v2/oauth2-provider/apps/{app} \
|
||||
{
|
||||
"callback_url": "string",
|
||||
"client_type": "confidential",
|
||||
"dynamically_registered": true,
|
||||
"endpoints": {
|
||||
"authorization": "string",
|
||||
"device_authorization": "string",
|
||||
|
||||
Generated
+12
-10
@@ -11033,6 +11033,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith
|
||||
{
|
||||
"callback_url": "string",
|
||||
"client_type": "confidential",
|
||||
"dynamically_registered": true,
|
||||
"endpoints": {
|
||||
"authorization": "string",
|
||||
"device_authorization": "string",
|
||||
@@ -11051,16 +11052,17 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith
|
||||
|
||||
### Properties
|
||||
|
||||
| Name | Type | Required | Restrictions | Description |
|
||||
|-----------------|------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| `callback_url` | string | false | | Deprecated: equal to the first entry of redirect_uris. Read redirect_uris instead. |
|
||||
| `client_type` | [codersdk.OAuth2ClientType](#codersdkoauth2clienttype) | false | | Client type is "confidential" or "public". |
|
||||
| `endpoints` | [codersdk.OAuth2AppEndpoints](#codersdkoauth2appendpoints) | false | | Endpoints are included in the app response for easier discovery. The OAuth2 spec does not have a defined place to find these (for comparison, OIDC has a '/.well-known/openid-configuration' endpoint). |
|
||||
| `icon` | string | false | | |
|
||||
| `id` | string | false | | |
|
||||
| `name` | string | false | | |
|
||||
| `redirect_uris` | array of string | false | | Redirect uris are the app's registered redirect URIs, primary first. |
|
||||
| `scope` | string | false | | Scope is the space-separated list of scopes this app's tokens may be granted. Empty means unrestricted. A non-empty value with no names is a configured allowlist that grants nothing. |
|
||||
| Name | Type | Required | Restrictions | Description |
|
||||
|--------------------------|------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| `callback_url` | string | false | | Deprecated: equal to the first entry of redirect_uris. Read redirect_uris instead. |
|
||||
| `client_type` | [codersdk.OAuth2ClientType](#codersdkoauth2clienttype) | false | | Client type is "confidential" or "public". |
|
||||
| `dynamically_registered` | boolean | false | | Dynamically registered is true when the app registered itself through Dynamic Client Registration rather than being created by an admin. |
|
||||
| `endpoints` | [codersdk.OAuth2AppEndpoints](#codersdkoauth2appendpoints) | false | | Endpoints are included in the app response for easier discovery. The OAuth2 spec does not have a defined place to find these (for comparison, OIDC has a '/.well-known/openid-configuration' endpoint). |
|
||||
| `icon` | string | false | | |
|
||||
| `id` | string | false | | |
|
||||
| `name` | string | false | | |
|
||||
| `redirect_uris` | array of string | false | | Redirect uris are the app's registered redirect URIs, primary first. |
|
||||
| `scope` | string | false | | Scope is the space-separated list of scopes this app's tokens may be granted. Empty means unrestricted. A non-empty value with no names is a configured allowlist that grants nothing. |
|
||||
|
||||
## codersdk.OAuth2ProviderAppSecret
|
||||
|
||||
|
||||
Generated
+5
@@ -6772,6 +6772,11 @@ export interface OAuth2ProviderApp {
|
||||
* ClientType is "confidential" or "public".
|
||||
*/
|
||||
readonly client_type: OAuth2ClientType;
|
||||
/**
|
||||
* DynamicallyRegistered is true when the app registered itself through
|
||||
* Dynamic Client Registration rather than being created by an admin.
|
||||
*/
|
||||
readonly dynamically_registered: boolean;
|
||||
/**
|
||||
* Endpoints are included in the app response for easier discovery. The OAuth2
|
||||
* spec does not have a defined place to find these (for comparison, OIDC has
|
||||
|
||||
+1
@@ -172,6 +172,7 @@ export const DynamicallyRegisteredValues: Story = {
|
||||
name: "VS Code Coder Extension",
|
||||
callback_url: "vscode://coder.coder-remote/oauth/callback",
|
||||
redirect_uris: ["vscode://coder.coder-remote/oauth/callback"],
|
||||
dynamically_registered: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -191,6 +191,12 @@ export const EditOAuth2AppPageView: FC = () => {
|
||||
value={app.endpoints.authorization}
|
||||
/>
|
||||
<EndpointField label="Token URL" value={app.endpoints.token} />
|
||||
<div className="flex items-center gap-2">
|
||||
<dt className="text-sm">Registration</dt>
|
||||
<dd className="m-0 text-sm text-content-secondary">
|
||||
{app.dynamically_registered ? "Self-registered" : "Admin-created"}
|
||||
</dd>
|
||||
</div>
|
||||
</dl>
|
||||
|
||||
{secretsQuery.error ? (
|
||||
|
||||
+43
@@ -5,6 +5,7 @@ import { externalScopesKey } from "#/api/queries/oauth2";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import {
|
||||
MockExternalAPIKeyScopes,
|
||||
MockOAuth2ProviderAppDynamic,
|
||||
MockOAuth2ProviderApps,
|
||||
mockApiError,
|
||||
} from "#/testHelpers/entities";
|
||||
@@ -93,3 +94,45 @@ export const Disabled: Story = {
|
||||
disabled: true,
|
||||
},
|
||||
};
|
||||
|
||||
export const SelfRegisteredScopesNarrowed: Story = {
|
||||
args: {
|
||||
app: {
|
||||
...MockOAuth2ProviderAppDynamic,
|
||||
scope: "coder:workspaces.access workspace:ssh",
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
await userEvent.click(
|
||||
within(canvasElement).getAllByTestId("clear-option-button")[0],
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export const AdminCreatedScopesNarrowed: Story = {
|
||||
args: { app: appWithScopes },
|
||||
play: async ({ canvasElement }) => {
|
||||
await userEvent.click(
|
||||
within(canvasElement).getAllByTestId("clear-option-button")[0],
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export const SelfRegisteredScopesWidened: Story = {
|
||||
args: {
|
||||
app: {
|
||||
...MockOAuth2ProviderAppDynamic,
|
||||
scope: "coder:workspaces.access workspace:ssh",
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
await userEvent.click(
|
||||
within(canvasElement).getByRole("combobox", { name: /allowed scopes/i }),
|
||||
);
|
||||
await userEvent.click(
|
||||
await within(canvasElement).findByRole("option", {
|
||||
name: "workspace:read",
|
||||
}),
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
+89
-2
@@ -14,7 +14,7 @@ import {
|
||||
MockOAuth2ProviderApps,
|
||||
} from "#/testHelpers/entities";
|
||||
import { render } from "#/testHelpers/renderHelpers";
|
||||
import { OAuth2AppForm } from "./OAuth2AppForm";
|
||||
import { narrowsAllowlist, OAuth2AppForm } from "./OAuth2AppForm";
|
||||
|
||||
const selectScope = async (user: UserEvent, name: string) => {
|
||||
await user.click(screen.getByRole("combobox", { name: /allowed scopes/i }));
|
||||
@@ -67,7 +67,7 @@ describe("OAuth2AppForm", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("submits edited dynamically registered client values", async () => {
|
||||
it("trims the edited name on update", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onSubmit = vi.fn();
|
||||
const app = {
|
||||
@@ -1021,3 +1021,90 @@ describe("OAuth2AppForm", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("narrowsAllowlist", () => {
|
||||
const catalog = MockExternalAPIKeyScopes.external;
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "adding a scope to an existing list",
|
||||
stored: "workspace:ssh",
|
||||
next: ["workspace:ssh", "workspace:read"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "removing a scope from an existing list",
|
||||
stored: "workspace:ssh workspace:read",
|
||||
next: ["workspace:read"],
|
||||
narrows: true,
|
||||
},
|
||||
{
|
||||
label: "swapping one scope for another",
|
||||
stored: "workspace:ssh",
|
||||
next: ["workspace:read"],
|
||||
narrows: true,
|
||||
},
|
||||
{
|
||||
label: "clearing the list",
|
||||
stored: "workspace:ssh",
|
||||
next: [],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "imposing a list on an unrestricted app",
|
||||
stored: "",
|
||||
next: ["workspace:ssh"],
|
||||
narrows: true,
|
||||
},
|
||||
{
|
||||
label: "imposing a list of unknown names on an unrestricted app",
|
||||
stored: "",
|
||||
next: ["legacy:thing"],
|
||||
narrows: true,
|
||||
},
|
||||
{
|
||||
label: "replacing a scope with coder:all",
|
||||
stored: "workspace:ssh",
|
||||
next: ["coder:all"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "imposing coder:all on an unrestricted app",
|
||||
stored: "",
|
||||
next: ["coder:all"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "keeping the same list",
|
||||
stored: "workspace:ssh",
|
||||
next: ["workspace:ssh"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "removing a name the deployment does not offer",
|
||||
stored: "workspace:ssh legacy:thing",
|
||||
next: ["workspace:ssh"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "removing the only offered name",
|
||||
stored: "workspace:ssh legacy:thing",
|
||||
next: ["legacy:thing"],
|
||||
narrows: true,
|
||||
},
|
||||
{
|
||||
label: "replacing a list that grants nothing",
|
||||
stored: "legacy:thing",
|
||||
next: ["workspace:read"],
|
||||
narrows: false,
|
||||
},
|
||||
{
|
||||
label: "adding a scope to a whitespace-only list",
|
||||
stored: " ",
|
||||
next: ["workspace:read"],
|
||||
narrows: false,
|
||||
},
|
||||
])("returns $narrows when $label", ({ stored, next, narrows }) => {
|
||||
expect(narrowsAllowlist(stored, next, catalog)).toBe(narrows);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useFormik } from "formik";
|
||||
import { TriangleAlertIcon } from "lucide-react";
|
||||
import { type FC, useEffect, useRef } from "react";
|
||||
import { useQuery } from "react-query";
|
||||
import { Link } from "react-router";
|
||||
import { Link as RouterLink } from "react-router";
|
||||
import * as Yup from "yup";
|
||||
import { getErrorMessage } from "#/api/errors";
|
||||
import { getExternalScopes } from "#/api/queries/oauth2";
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
OAuth2RedirectURIMaxBytes,
|
||||
OAuth2RedirectURIsMaxCount,
|
||||
} from "#/api/typesGenerated";
|
||||
import { Alert } from "#/components/Alert/Alert";
|
||||
import { ErrorAlert } from "#/components/Alert/ErrorAlert";
|
||||
import { Button } from "#/components/Button/Button";
|
||||
import { ConfirmDialog } from "#/components/Dialog/ConfirmDialog/ConfirmDialog";
|
||||
@@ -19,9 +20,11 @@ import { Form, FormFields } from "#/components/Form/Form";
|
||||
import { FormField } from "#/components/FormField/FormField";
|
||||
import { IconField } from "#/components/IconField/IconField";
|
||||
import { Label } from "#/components/Label/Label";
|
||||
import { Link } from "#/components/Link/Link";
|
||||
import { MultiSelectCombobox } from "#/components/MultiSelectCombobox/MultiSelectCombobox";
|
||||
import { Spinner } from "#/components/Spinner/Spinner";
|
||||
import { useUnsavedChangesPrompt } from "#/hooks/useUnsavedChangesPrompt";
|
||||
import { docs } from "#/utils/docs";
|
||||
import { getFormHelpers, iconValidator } from "#/utils/formUtils";
|
||||
import { RedirectURIsField } from "./components/RedirectURIsField";
|
||||
|
||||
@@ -62,6 +65,40 @@ type OAuth2AppFormProps = OAuth2AppFormSubmit & {
|
||||
const BACK_HREF = "/deployment/oauth2-provider/apps";
|
||||
const SCOPE_LABEL = "Allowed scopes";
|
||||
|
||||
// Typed so a renamed scope fails to compile instead of silently not matching.
|
||||
const coderAll: TypesGen.APIKeyScope = "coder:all";
|
||||
|
||||
/**
|
||||
* narrowsAllowlist reports whether replacing the stored scope allowlist with
|
||||
* next can reject a scope the client could previously be granted. It follows
|
||||
* the server in ignoring names outside the catalog. It cannot see which scopes
|
||||
* cover others beyond coder:all, so it may warn about a change the server
|
||||
* would accept, but it never stays quiet about one the server would reject.
|
||||
*/
|
||||
export const narrowsAllowlist = (
|
||||
stored: string,
|
||||
next: readonly string[],
|
||||
catalog: readonly string[],
|
||||
): boolean => {
|
||||
if (next.length === 0) {
|
||||
return false;
|
||||
}
|
||||
const nextGrantable = next.filter((scope) => catalog.includes(scope));
|
||||
if (nextGrantable.includes(coderAll)) {
|
||||
return false;
|
||||
}
|
||||
if (stored === "") {
|
||||
return true;
|
||||
}
|
||||
const storedGrantable = stored
|
||||
.split(" ")
|
||||
.filter((scope) => catalog.includes(scope));
|
||||
if (storedGrantable.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return storedGrantable.some((scope) => !nextGrantable.includes(scope));
|
||||
};
|
||||
|
||||
// Mirror codersdk.ValidateRedirectURIShape.
|
||||
// The server remains authoritative for URL syntax differences between parsers.
|
||||
// oxlint-disable-next-line eslint/no-script-url -- This blocklist rejects the scheme; it is never used as a navigation target.
|
||||
@@ -264,6 +301,12 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
|
||||
: undefined;
|
||||
const formDisabled = disabled || isUpdating;
|
||||
const editing = Boolean(app);
|
||||
// The Scopes section of docs/admin/integrations/oauth2-provider.md explains
|
||||
// why narrowing a self-registered application's allowlist can break it.
|
||||
const narrowsSelfRegisteredScopes =
|
||||
app?.dynamically_registered === true &&
|
||||
scopesQuery.data !== undefined &&
|
||||
narrowsAllowlist(app.scope, form.values.scope, scopesQuery.data.external);
|
||||
const submitDisabled =
|
||||
formDisabled || !form.isValid || (editing && !form.dirty);
|
||||
|
||||
@@ -375,6 +418,21 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
|
||||
granted. Empty means no restriction: tokens can be granted any
|
||||
scope.
|
||||
</div>
|
||||
{narrowsSelfRegisteredScopes && (
|
||||
<Alert severity="warning">
|
||||
A self-registered client that requests every advertised scope
|
||||
fails new authorizations with invalid_scope once its allowlist is
|
||||
narrower than that list. Existing tokens keep their scopes.{" "}
|
||||
<Link
|
||||
href={docs("/admin/integrations/oauth2-provider#scopes")}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
>
|
||||
Learn more
|
||||
<span className="sr-only"> (link opens in new tab)</span>
|
||||
</Link>
|
||||
</Alert>
|
||||
)}
|
||||
<MultiSelectCombobox
|
||||
// cmdk generates the input's id and aria-labelledby itself, so
|
||||
// the accessible name has to come from its own label prop.
|
||||
@@ -432,7 +490,7 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
|
||||
|
||||
<div className="flex justify-end gap-4">
|
||||
<Button variant="outline" asChild>
|
||||
<Link to={BACK_HREF}>Cancel</Link>
|
||||
<RouterLink to={BACK_HREF}>Cancel</RouterLink>
|
||||
</Button>
|
||||
<Button disabled={submitDisabled} type="submit">
|
||||
<Spinner loading={isUpdating} />
|
||||
|
||||
@@ -5010,6 +5010,7 @@ export const MockOAuth2ProviderApps: TypesGen.OAuth2ProviderApp[] = [
|
||||
icon: "/icon/github.svg",
|
||||
scope: "",
|
||||
client_type: "confidential",
|
||||
dynamically_registered: false,
|
||||
endpoints: {
|
||||
authorization: "http://127.0.0.1:3001/oauth2/authorize",
|
||||
token: "http://127.0.0.1:3001/oauth2/token",
|
||||
@@ -5027,6 +5028,7 @@ export const MockOAuth2ProviderAppPublic: TypesGen.OAuth2ProviderApp = {
|
||||
icon: "/icon/github.svg",
|
||||
scope: "",
|
||||
client_type: "public",
|
||||
dynamically_registered: false,
|
||||
endpoints: {
|
||||
authorization: "http://127.0.0.1:3002/oauth2/authorize",
|
||||
token: "http://127.0.0.1:3002/oauth2/token",
|
||||
@@ -5035,6 +5037,11 @@ export const MockOAuth2ProviderAppPublic: TypesGen.OAuth2ProviderApp = {
|
||||
},
|
||||
};
|
||||
|
||||
export const MockOAuth2ProviderAppDynamic: TypesGen.OAuth2ProviderApp = {
|
||||
...MockOAuth2ProviderApps[0],
|
||||
dynamically_registered: true,
|
||||
};
|
||||
|
||||
export const MockOAuth2ProviderSettings: TypesGen.OAuth2ProviderSettings = {
|
||||
dynamic_client_registration_enabled: false,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user