diff --git a/docs/configuration/uds-operator.md b/docs/configuration/uds-operator.md index 87d340d70..bc4163428 100644 --- a/docs/configuration/uds-operator.md +++ b/docs/configuration/uds-operator.md @@ -168,6 +168,29 @@ variables: See [configuring Istio Ingress](https://uds.defenseunicorns.com/core/configuration/istio/ingress/#configure-domain-name-and-tls-for-istio-gateways) for the relevant documentation on configuring ingress certificates. +### Creating a UDS Package with a Device Flow client + +Some applications may not have a web UI / server component to login to and may instead grant OAuth tokens to devices. This flow is known as the [OAuth 2.0 Device Authorization Grant](https://oauth.net/2/device-flow/) and is supported in a UDS Package with the following configuration: + +```yaml +apiVersion: uds.dev/v1alpha1 +kind: Package +metadata: + name: fulcio + namespace: fulcio-system +spec: + sso: + sso: + - name: Sigstore Login + clientId: sigstore + standardFlowEnabled: false + publicClient: true + attributes: + oauth2.device.authorization.grant.enabled: "true" +``` + +This configuration does not create a secret in the cluster and instead tells the UDS Operator to create a public client (one that requires no auth secret) that enables the `oauth2.device.authorization.grant.enabled` flow and disables the standard redirect auth flow. Because this creates a public client configuration that deviates from this is limited - if your application requires both the Device Authorization Grant and the standard flow this is currently not supported without creating two separate clients. + ## Exemption - **Exemption Scope:** diff --git a/src/pepr/operator/controllers/keycloak/client-sync.ts b/src/pepr/operator/controllers/keycloak/client-sync.ts index ddc17d264..169fd2a66 100644 --- a/src/pepr/operator/controllers/keycloak/client-sync.ts +++ b/src/pepr/operator/controllers/keycloak/client-sync.ts @@ -158,22 +158,24 @@ async function syncClient( } // Create or update the client secret - const generation = (pkg.metadata?.generation ?? 0).toString(); - await K8s(kind.Secret).Apply({ - metadata: { - namespace: pkg.metadata!.namespace, - // Use the CR secret name if provided, otherwise use the client name - name: secretName || name, - labels: { - "uds/package": pkg.metadata!.name, - "uds/generation": generation, + if (!client.publicClient) { + const generation = (pkg.metadata?.generation ?? 0).toString(); + await K8s(kind.Secret).Apply({ + metadata: { + namespace: pkg.metadata!.namespace, + // Use the CR secret name if provided, otherwise use the client name + name: secretName || name, + labels: { + "uds/package": pkg.metadata!.name, + "uds/generation": generation, + }, + + // Use the CR as the owner ref for each VirtualService + ownerReferences: getOwnerRef(pkg), }, - - // Use the CR as the owner ref for each VirtualService - ownerReferences: getOwnerRef(pkg), - }, - data: generateSecretData(client, secretTemplate), - }); + data: generateSecretData(client, secretTemplate), + }); + } return client; } diff --git a/src/pepr/operator/crd/generated/package-v1alpha1.ts b/src/pepr/operator/crd/generated/package-v1alpha1.ts index a96450297..cd0f7330c 100644 --- a/src/pepr/operator/crd/generated/package-v1alpha1.ts +++ b/src/pepr/operator/crd/generated/package-v1alpha1.ts @@ -549,11 +549,15 @@ export interface Sso { * Specifies the protocol of the client, either 'openid-connect' or 'saml' */ protocol?: Protocol; + /** + * Defines whether the client requires a client secret for authentication + */ + publicClient?: boolean; /** * Valid URI pattern a browser can redirect to after a successful login. Simple wildcards * are allowed such as 'https://unicorns.uds.dev/*' */ - redirectUris: string[]; + redirectUris?: string[]; /** * Root URL appended to relative URLs */ @@ -570,6 +574,10 @@ export interface Sso { * A template for the generated secret */ secretTemplate?: { [key: string]: string }; + /** + * Enables the standard OpenID Connect redirect based authentication with authorization code. + */ + standardFlowEnabled?: boolean; /** * Allowed CORS origins. To permit all origins of Valid Redirect URIs, add '+'. This does * not include the '*' wildcard though. To permit all origins, explicitly add '*'. diff --git a/src/pepr/operator/crd/index.ts b/src/pepr/operator/crd/index.ts index 285c6c904..e4f786f2f 100644 --- a/src/pepr/operator/crd/index.ts +++ b/src/pepr/operator/crd/index.ts @@ -8,6 +8,7 @@ export { Status as PkgStatus, RemoteGenerated, Sso, + Protocol, Package as UDSPackage, } from "./generated/package-v1alpha1"; diff --git a/src/pepr/operator/crd/sources/package/v1alpha1.ts b/src/pepr/operator/crd/sources/package/v1alpha1.ts index e5628b230..d228a0c6b 100644 --- a/src/pepr/operator/crd/sources/package/v1alpha1.ts +++ b/src/pepr/operator/crd/sources/package/v1alpha1.ts @@ -252,7 +252,7 @@ const sso = { type: "array", items: { type: "object", - required: ["clientId", "name", "redirectUris"], + required: ["clientId", "name"], properties: { enableAuthserviceSelector: { description: @@ -335,6 +335,17 @@ const sso = { type: "boolean", default: false, }, + standardFlowEnabled: { + description: + "Enables the standard OpenID Connect redirect based authentication with authorization code.", + type: "boolean", + default: true, + }, + publicClient: { + description: "Defines whether the client requires a client secret for authentication", + type: "boolean", + default: false, + }, clientAuthenticatorType: { description: "The client authenticator type", type: "string", diff --git a/src/pepr/operator/crd/validators/package-validator.spec.ts b/src/pepr/operator/crd/validators/package-validator.spec.ts new file mode 100644 index 000000000..daa3d7035 --- /dev/null +++ b/src/pepr/operator/crd/validators/package-validator.spec.ts @@ -0,0 +1,398 @@ +import { afterEach, describe, expect, it, jest } from "@jest/globals"; +import { PeprValidateRequest } from "pepr"; +import { Gateway, Expose, UDSPackage, Allow, Sso, Direction, RemoteGenerated, Protocol } from ".."; +import { validator } from "./package-validator"; + +const makeMockReq = ( + pkg: Partial, + exposeList: Partial[], + allowList: Partial[], + ssoClients: Partial[], +) => { + const defaultPkg: UDSPackage = { + metadata: { + namespace: "application-system", + name: "application", + }, + spec: { + network: { + expose: [], + allow: [], + }, + sso: [], + }, + }; + + for (const expose of exposeList) { + const defaultExpose: Expose = { + host: "app", + }; + defaultPkg.spec!.network!.expose?.push({ ...defaultExpose, ...expose }); + } + + for (const allow of allowList) { + const defaultAllow: Allow = { + direction: Direction.Egress, + }; + defaultPkg.spec!.network!.allow?.push({ ...defaultAllow, ...allow }); + } + + for (const client of ssoClients) { + const defaultClient: Sso = { + name: "Application Login", + clientId: "uds-package-application", + redirectUris: ["https://app.uds.dev/redirect"], + }; + defaultPkg.spec!.sso?.push({ ...defaultClient, ...client }); + } + + return { + Raw: { ...defaultPkg, ...pkg }, + Approve: jest.fn(), + Deny: jest.fn(), + } as unknown as PeprValidateRequest; +}; + +describe("Test validation of Exemption CRs", () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it("allows packages that have no issues", async () => { + const mockReq = makeMockReq({}, [{}], [{}], [{}]); + await validator(mockReq); + expect(mockReq.Approve).toHaveBeenCalledTimes(1); + }); + + it("denies system namespaces", async () => { + const mockReq = makeMockReq({ metadata: { namespace: "kube-system" } }, [], [], []); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies advancedHTTP when used with passthrough Gateways", async () => { + const mockReq = makeMockReq( + {}, + [ + { + gateway: Gateway.Passthrough, + advancedHTTP: { + directResponse: { status: 403 }, + }, + }, + ], + [], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies advancedHTTP.directResponse when used with a selector", async () => { + const mockReq = makeMockReq( + {}, + [ + { + advancedHTTP: { + directResponse: { status: 403 }, + }, + selector: { app: "app" }, + }, + ], + [], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies advancedHTTP.directResponse when used with a service", async () => { + const mockReq = makeMockReq( + {}, + [ + { + advancedHTTP: { + directResponse: { status: 403 }, + }, + service: "app-service", + }, + ], + [], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies advancedHTTP.directResponse when used with a port", async () => { + const mockReq = makeMockReq( + {}, + [ + { + advancedHTTP: { + directResponse: { status: 403 }, + }, + port: 443, + }, + ], + [], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies advancedHTTP.directResponse when used with a targetPort", async () => { + const mockReq = makeMockReq( + {}, + [ + { + advancedHTTP: { + directResponse: { status: 403 }, + }, + port: 8443, + }, + ], + [], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies virtual services that are the same name", async () => { + const mockReq = makeMockReq({}, [{}, {}], [], []); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies network policies that specify both remoteGenerated and remoteNamespace", async () => { + const mockReq = makeMockReq( + {}, + [], + [ + { + remoteGenerated: RemoteGenerated.Anywhere, + remoteNamespace: "other-system", + }, + ], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies network policies that specify both remoteGenerated and remoteSelector", async () => { + const mockReq = makeMockReq( + {}, + [], + [ + { + remoteGenerated: RemoteGenerated.Anywhere, + remoteSelector: { app: "other" }, + }, + ], + [], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies network policies that are the same name", async () => { + const mockReq = makeMockReq({}, [], [{}, {}], []); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies clients with clientIDs that are not unique", async () => { + const mockReq = makeMockReq({}, [], [], [{}, {}]); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies clients with invalid secret names", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + secretName: "HELLO_KITTEH", + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies clients with using the standard flow that don't have redirectUris", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + redirectUris: undefined, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("allows clients not using the standard flow that don't have redirectUris", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + standardFlowEnabled: false, + redirectUris: undefined, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Approve).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using the standard flow", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: true, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using a secret", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + secret: "app-client-secret", + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using a secretName", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + secretName: "app-k8s-secret", + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using a secretTemplate", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + secretTemplate: {}, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using enableAuthserviceSelector", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + enableAuthserviceSelector: {}, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public device flow clients using the saml protocol", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + protocol: Protocol.Saml, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("denies public clients without the device flow attribute", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + standardFlowEnabled: false, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(1); + }); + + it("allows public clients that have the device flow attribute with standard flow disabled", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [ + { + publicClient: true, + attributes: { "oauth2.device.authorization.grant.enabled": "true" }, + standardFlowEnabled: false, + }, + ], + ); + await validator(mockReq); + expect(mockReq.Approve).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/pepr/operator/crd/validators/package-validator.ts b/src/pepr/operator/crd/validators/package-validator.ts index 2c8955059..a7684c10a 100644 --- a/src/pepr/operator/crd/validators/package-validator.ts +++ b/src/pepr/operator/crd/validators/package-validator.ts @@ -1,6 +1,6 @@ import { PeprValidateRequest } from "pepr"; -import { Gateway, UDSPackage } from ".."; +import { Gateway, Protocol, UDSPackage } from ".."; import { generateVSName } from "../../controllers/istio/virtual-service"; import { generateName } from "../../controllers/network/generate"; import { sanitizeResourceName } from "../../controllers/utils"; @@ -92,6 +92,27 @@ export async function validator(req: PeprValidateRequest) { `The client ID "${client.clientId}" uses an invalid secret name ${client.secretName}`, ); } + // If standardFlowEnabled is undefined (defaults to `true`) or explicitly true and there are no redirectUris set, deny the req + if (client.standardFlowEnabled !== false && !client.redirectUris) { + return req.Deny( + `The client ID "${client.clientId}" must specify redirectUris if standardFlowEnabled is turned on (it is enabled by default)`, + ); + } + // If this is a public client ensure that it only sets itself up as an OAuth Device Flow client + if ( + client.publicClient && + (client.standardFlowEnabled !== false || + client.secret !== undefined || + client.secretName !== undefined || + client.secretTemplate !== undefined || + client.enableAuthserviceSelector !== undefined || + client.protocol === Protocol.Saml || + client.attributes?.["oauth2.device.authorization.grant.enabled"] !== "true") + ) { + return req.Deny( + `The client ID "${client.clientId}" must _only_ configure the OAuth Device Flow as a public client`, + ); + } } return req.Approve();