Skip to content

Commit d060533

Browse files
authored
OIDC validation & issuer fixes (#363)
* fix: validation and issuer checks * feat: query param util * fix: lint
1 parent 2d2b815 commit d060533

3 files changed

Lines changed: 28 additions & 16 deletions

File tree

server/internal/auth/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ class AuthManager {
3030
(this.authProviders as any)[key] = object;
3131
logger.info(`enabled auth: ${key}`);
3232
} catch (e) {
33-
logger.warn((e as string).toString());
33+
logger.warn(
34+
`failed to enable auth ${key}: ${(e as string).toString()}`,
35+
);
3436
}
3537
}
3638

server/internal/auth/oidc/index.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,22 @@ import * as jose from "jose";
1212
// import { inspect } from "util";
1313
import sessionHandler from "../../session";
1414
import type { SessionSearchTerms } from "../../session/types";
15+
import { queryParamBuilder } from "../../utils/query";
1516

1617
// TODO: monitor https://github.com/goauthentik/authentik/issues/8751 for easier?? OIDC setup by end users
1718

1819
// Schema for OIDC well-known configuration
1920
const OIDCWellKnownV1 = type({
20-
issuer: "string.url.parse",
21+
issuer: "string",
2122
authorization_endpoint: "string.url.parse",
2223
token_endpoint: "string.url.parse",
23-
userinfo_endpoint: "string.url.parse?",
24+
userinfo_endpoint: "string.url.parse",
2425
jwks_uri: "string.url.parse",
25-
scopes_supported: "string[]?",
26+
scopes_supported: "string[]",
2627
});
2728

2829
// Represents required OIDC configuration
29-
interface OIDCConfiguration {
30-
issuer: URL;
31-
authorization_endpoint: URL;
32-
token_endpoint: URL;
33-
userinfo_endpoint: URL;
34-
jwks_uri: URL;
35-
scopes_supported: string[];
36-
}
30+
type OIDCConfiguration = typeof OIDCWellKnownV1.infer;
3731

3832
interface OIDCAuthSessionOptions {
3933
redirect: string | undefined;
@@ -241,7 +235,7 @@ export class OIDCManager {
241235
token_endpoint: new URL(tokenEndpoint),
242236
userinfo_endpoint: new URL(userinfoEndpoint),
243237
scopes_supported: scopes.split(","),
244-
issuer: new URL(issuer),
238+
issuer: issuer,
245239
jwks_uri: new URL(jwksEndpoint),
246240
};
247241
}
@@ -294,7 +288,15 @@ export class OIDCManager {
294288
this.oidcConfiguration.authorization_endpoint,
295289
).toString();
296290

297-
const finalUrl = `${normalisedUrl}?client_id=${this.clientId}&redirect_uri=${encodeURIComponent(this.redirectUrl.toString())}&state=${stateKey}&response_type=code&scope=${encodeURIComponent(this.oidcConfiguration.scopes_supported.join(" "))}`;
291+
const queryParams = queryParamBuilder({
292+
client_id: this.clientId,
293+
redirect_uri: this.redirectUrl.toString(),
294+
state: stateKey,
295+
response_type: "code",
296+
scope: this.oidcConfiguration.scopes_supported.join(" "),
297+
});
298+
299+
const finalUrl = `${normalisedUrl}?${queryParams}`;
298300

299301
const session: OIDCAuthSession = {
300302
redirectUrl: finalUrl,
@@ -549,7 +551,8 @@ export class OIDCManager {
549551
}
550552
}
551553

552-
function isHttps(url: URL): boolean {
553-
if (url.protocol === "https:") return true;
554+
function isHttps(url: URL | string): boolean {
555+
const parsedUrl = typeof url === "string" ? new URL(url) : url;
556+
if (parsedUrl.protocol === "https:") return true;
554557
else return false;
555558
}

server/internal/utils/query.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export function queryParamBuilder(params: { [key: string]: string }) {
2+
const list = Object.entries(params).map(
3+
([key, value]) => `${key}=${encodeURIComponent(value)}`,
4+
);
5+
const str = list.join("&");
6+
return str;
7+
}

0 commit comments

Comments
 (0)