Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,29 @@
# Can be generated with e.g. pwgen -s 64 1
# Please provide a string of length 64+ characters
# SESSION_SECRET=

# Enable login with OAuth 2.0
# OAUTH2_ENABLED: false
# OAUTH2_CLIENT_ID: ctfnote
# OAUTH2_CLIENT_SECRET: insecure_secret
# OAUTH2_SCOPE: openid profile groups

# The attribute to use as login and username
# OAUTH2_USERNAME_ATTR: name

# The attribute to use for determining the user's role
# The attribute can either be a string or an array of strings
# In case of an array, the highest role will be selected
# OAUTH2_ROLE_ATTR: groups

# A mapping for the values of the attribute to roles in CTFNote
# roles: user_admin, user_manager, user_member, user_fried, user_guest, none (no access to CTFNote)
# OAUTH2_ROLE_MAPPING: '{"admin": "user_admin"}'

# Either specify the discovery url or all other properties
# If a discovery url is provided, the other properties overwrite the values from the discovery
# OAUTH2_DISCOVERY_URL: https://example.com/.well-known/openid-configuration
# OAUTH2_ISSUER: https://example.com
# OAUTH2_AUTHORIZATION_ENDPOINT: https://example.com/api/oidc/authorization
# OAUTH2_TOKEN_ENDPOINT: https://example.com/api/oidc/token
# OAUTH2_USERINFO_ENDPOINT: https://example.com/api/oidc/userinfo
Binary file not shown.
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Global args, set before the first FROM, shared by all stages
ARG NODE_ENV="production"
ARG NODE_IMAGE="18.19.1-alpine"
ARG NODE_IMAGE="20.19.5-alpine"

################################################################################
# Build stage 1 - `yarn build`
Expand Down
25 changes: 25 additions & 0 deletions api/migrations/57-external-login.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-- Login with external identity and register/migrate user if not present/externally managed
CREATE FUNCTION ctfnote_private.login_with_extern("name" text, "role" ctfnote.role)
RETURNS ctfnote.jwt
AS $$
DECLARE
log_user ctfnote_private.user;
BEGIN
INSERT INTO ctfnote_private.user ("login", "password", "role")
VALUES (login_with_extern.name, 'external', login_with_extern.role)
ON CONFLICT ("login") DO UPDATE
SET password = 'external', role = login_with_extern.role
Comment on lines +10 to +11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you login with an external identity provider, you will note be able to also do password login? Why can't it be both? So that you set a password after doing a password reset in CTFNote and then you choose to use OAuth login or through your password and you enter the same account.
For example, we can initially set the password to null to prevent any authentication through password and ON CONFLICT here we just do nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is definitively an option (after dropping the NOT NULL constraint).
But in that case we would end up with the opposite situation: I have a local account and want to migrate it to external only. With that option, the best I can do is to choose a long, random password and immediately forget it. Since that is less impacting, I probably will change it that way in the upcoming days.

RETURNING
* INTO log_user;
INSERT INTO ctfnote.profile ("id", "username")
VALUES (log_user.id, login_with_extern.name)
ON CONFLICT (id) DO UPDATE
SET username = login_with_extern.name;
RETURN (ctfnote_private.new_token (log_user.id))::ctfnote.jwt;
END;
$$
LANGUAGE plpgsql
STRICT
SECURITY DEFINER;

GRANT EXECUTE ON FUNCTION ctfnote_private.login_with_extern TO user_anonymous;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would a random online user prevent to call this function to login as another user? Can I just call this function with the name of the administrator, or just give me a high privilege role? This function looks really vulnerable...

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm maybe because it is in the ctfnote_private space it is still correct and not publicly exposed. I mostly forgot how this system works... This pattern is used on other places in the code too, but still I am looking forward to get a clarification from you :)

Copy link
Contributor Author

@Minei3oat Minei3oat Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my first time working with graphql. As a result, I can't give a definitive answer. I created the graphql parts by immitating similar code in ctfnote.
But I think I mitigated the flaw in #509 by changing the function to ctfnote_private, suggesting that ctfnote_private is indeed not exposed externally.

Copy link
Contributor Author

@Minei3oat Minei3oat Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I make the function public, the following payload returns a JWT:

[{"operationName":"loginWithExtern","variables":{"name":"test","role":"USER_ADMIN"},"query":"mutation loginWithExtern($name: String!, $role: Role!) {\n  loginWithExtern(input: {name: $name, role: $role}) {\n    jwt\n    __typename\n  }\n}"}]

But that payload does not work if the function is private. But maybe my graphql is just to primitive to come up with a working version.

Copy link
Contributor Author

@Minei3oat Minei3oat Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit more searching, I found that ctfnote_private.new_token ("user_id" int) (https://github.com/TFNS/CTFNote/blob/main/api/migrations/9-type-jwt.sql#L90-L102) was executable by anonymous, too. It was changed in bab8767 due to a refactoring, but I wasn't able to find whether it was for a security fix or just some functional improvement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to correct myself: That function still exists.
As a result, it is much more straight forward to directly request the JWT of an existing admin instead of creating a new user.

5 changes: 5 additions & 0 deletions api/migrations/58-oauth2-login.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE ctfnote.settings
ADD COLUMN "oauth2_enabled" boolean NOT NULL DEFAULT FALSE;

GRANT SELECT ("oauth2_enabled") ON ctfnote.settings TO user_anonymous;
GRANT UPDATE ("oauth2_enabled") ON ctfnote.settings TO user_postgraphile;
1 change: 1 addition & 0 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"graphql": "^16.9.0",
"graphql-upload-ts": "^2.1.2",
"ical-generator": "^7.0.0",
"openid-client": "6.8.1",
"postgraphile": "4.13.0",
"postgraphile-plugin-connection-filter": "^2.3.0",
"postgres-migrations": "^5.3.0",
Expand Down
33 changes: 33 additions & 0 deletions api/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,21 @@ export type CTFNoteConfig = DeepReadOnly<{
registrationRoleId: string;
channelHandleStyle: DiscordChannelHandleStyle;
};
oauth2: {
enabled: string;
clientId: string;
clientSecret: string;
scope: string;
usernameAttr: string;
roleAttr: string;
roleMapping: string;
discoveryUrl: string;
authorizationEndpoint: string;
tokenEndpoint: string;
userinfoEndpoint: string;
tokenEndpointAuthMethod: string;
issuer: string;
};
}>;

function getEnv(
Expand Down Expand Up @@ -112,6 +127,24 @@ const config: CTFNoteConfig = {
"agile"
) as DiscordChannelHandleStyle,
},
oauth2: {
enabled: getEnv("OAUTH2_ENABLED", "false"),
clientId: getEnv("OAUTH2_CLIENT_ID", ""),
clientSecret: getEnv("OAUTH2_CLIENT_SECRET", ""),
scope: getEnv("OAUTH2_SCOPE", ""),
usernameAttr: getEnv("OAUTH2_USERNAME_ATTR", ""),
roleAttr: getEnv("OAUTH2_ROLE_ATTR", ""),
roleMapping: getEnv("OAUTH2_ROLE_MAPPING", ""),
discoveryUrl: getEnv("OAUTH2_DISCOVERY_URL", ""),
authorizationEndpoint: getEnv("OAUTH2_AUTHORIZATION_ENDPOINT", ""),
tokenEndpoint: getEnv("OAUTH2_TOKEN_ENDPOINT", ""),
userinfoEndpoint: getEnv("OAUTH2_USERINFO_ENDPOINT", ""),
tokenEndpointAuthMethod: getEnv(
"OAUTH2_TOKEN_ENDPOINT_AUTH_METHOD",
"client_secret_basic"
),
issuer: getEnv("OAUTH2_ISSUER", ""),
},
};

export default config;
2 changes: 1 addition & 1 deletion api/src/discord/agile/commands/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import {
} from "discord.js";
import { Command } from "../../interfaces/command";
import {
AllowedRoles,
createInvitationTokenForDiscordId,
getInvitationTokenForDiscordId,
getUserByDiscordId,
} from "../../database/users";
import { AllowedRoles } from "../../../utils/role";
import config from "../../../config";

async function getInvitationUrl(invitationCode: string | null = null) {
Expand Down
10 changes: 1 addition & 9 deletions api/src/discord/database/users.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { connectToDatabase } from "../../utils/database";
import { PoolClient } from "pg";
import { AllowedRoles } from "../../utils/role";

/*
* Only returns users that have not linked their discord account yet.
Expand Down Expand Up @@ -45,15 +46,6 @@ export async function setDiscordIdForUser(
}
}

// refactor above to an enum
export enum AllowedRoles {
user_guest = "user_guest",
user_friend = "user_friend",
user_member = "user_member",
user_manager = "user_manager",
user_admin = "user_admin",
}

export async function getInvitationTokenForDiscordId(
discordId: string,
pgClient: PoolClient | null = null
Expand Down
13 changes: 11 additions & 2 deletions api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ import uploadLogoPlugin from "./plugins/uploadLogo";
import uploadScalar from "./plugins/uploadScalar";
import { Pool } from "pg";
import { icalRoute } from "./routes/ical";
import { oauth2Router } from "./routes/oauth2";
import ConnectionFilterPlugin from "postgraphile-plugin-connection-filter";
import OperationHook from "@graphile/operation-hooks";
import discordHooks from "./discord/hooks";
import { initDiscordBot } from "./discord";
import PgManyToManyPlugin from "@graphile-contrib/pg-many-to-many";
import ProfileSubscriptionPlugin from "./plugins/ProfileSubscriptionPlugin";
import {
checkOAuth2Enabled,
loginWithOAuth2Plugin,
} from "./plugins/loginWithOAuth2";

function getDbUrl(role: "user" | "admin") {
const login = config.db[role].login;
Expand Down Expand Up @@ -63,6 +68,7 @@ function createOptions() {
discordHooks,
PgManyToManyPlugin,
ProfileSubscriptionPlugin,
loginWithOAuth2Plugin,
],
ownerConnectionString: getDbUrl("admin"),
enableQueryBatching: true,
Expand Down Expand Up @@ -109,7 +115,7 @@ function createOptions() {
return postgraphileOptions;
}

function createApp(postgraphileOptions: PostGraphileOptions) {
async function createApp(postgraphileOptions: PostGraphileOptions) {
const pool = new Pool({
connectionString: getDbUrl("user"),
});
Expand All @@ -126,6 +132,9 @@ function createApp(postgraphileOptions: PostGraphileOptions) {
);
app.use(postgraphile(pool, "ctfnote", postgraphileOptions));
app.use("/calendar.ics", icalRoute(pool));
if (await checkOAuth2Enabled()) {
app.use("/api/auth/oauth2", oauth2Router);
}
return app;
}

Expand All @@ -150,7 +159,7 @@ async function main() {
return;
}
const postgraphileOptions = createOptions();
const app = createApp(postgraphileOptions);
const app = await createApp(postgraphileOptions);

await initDiscordBot();

Expand Down
Loading
Loading