progress

backend/package.json

@@ -43,6 +43,7 @@
         "graphql-request": "^6.1.0",
         "joi": "^17.13.3",
         "jsonwebtoken": "^9.0.2",
+        "jtd": "^0.1.1",
         "passport": "^0.7.0",
         "passport-atlassian-oauth2": "^2.1.0",
         "passport-github2": "^0.1.12",

backend/src/api-login/strategy/dto/get-strategy-instance-detail.dto.ts

@@ -34,58 +34,11 @@ export class StrategyInstanceDetailResponse {
      * {@link StrategyInstance.doesImplicitRegister}
      */
     doesImplicitRegister: boolean;
-
     /**
-     * All URLs that are relevant for the client whenusing this strategy instance to authenticte
+     * Instance config with sensitive data removed
+     * 
+     * {@link StrategyInstance.instanceConfig}
      */
-    urls: StrategyInstanceUrlsResponse;
+    instanceConfig: object;
 }
 
-/**
- * A response type containing url information for a strategy instance
- *
- * For every instance the redirect url is given, for instances allowing post credentials it also contains the post urls
- */
-export class StrategyInstanceUrlsResponse {
-    /**
-     * The url to post credentials to, to log in using this strategy instance.
-     *
-     * Additionally `grant_type? must be set to "password" and the client must authenticate
-     */
-    postLogin?: string;
-
-    /**
-     * The url to post credentials to, to register or link **without** sync using this strategy instance.
-     *
-     * Additionally `grant_type? must be set to "password" and the client must authenticate
-     */
-    postRegister?: string;
-
-    /**
-     * The url to post credentials to, to register or link **with** avtivated sync functionality
-     *
-     * Additionally `grant_type? must be set to "password" and the client must authenticate
-     */
-    postRegisterSync?: string;
-
-    /**
-     * The url to redirect the user to, to log in using this strategy
-     *
-     * On the request, the query parameter `client_id` must be set
-     */
-    redirectLogin: string;
-
-    /**
-     * The url to redirect the user to, to register or link **without** sync using this strategy instance.
-     *
-     * On the request, the query parameter `client_id` must be set
-     */
-    redirectRegister: string;
-
-    /**
-     * The url to redirect the user to, to register or link **with** avtivated sync functionality
-     *
-     * On the request, the query parameter `client_id` must be set
-     */
-    redirectRegisterSync: string;
-}

backend/src/api-login/strategy/dto/strategy-api.dto.ts

@@ -1,3 +1,4 @@
+import { Schema } from "jtd";
 import { StrategyVariable } from "src/strategies/Strategy";
 
 /**
@@ -47,5 +48,10 @@ export class GetStrategyResponse {
      * The specification of the data expected in the post body if not using redirect
      * and sending the credentials directly to the token endpoint
      */
-    acceptsVariables: { [key: string]: StrategyVariable };
+    acceptsVariables: Record<string, StrategyVariable>;
+
+    /**
+     * The schema of the instance configuration
+     */
+    instanceConfigSchema: Record<string, Schema>;
 }

backend/src/api-login/strategy/dto/update-strategy-instance.dto.ts

@@ -8,7 +8,7 @@ export class UpdateStrategyInstanceInput {
      * The name for the strategy instance to create.
      * Can be left out.
      *
-     * If given, must be a non empty string matching `/^[a-zA-Z0-9+/\-_= ]+$/`
+     * If given, must be a non empty string
      *
      * @exmple "userpass-local"
      */
@@ -59,7 +59,7 @@ export class UpdateStrategyInstanceInput {
      * Checks, input is a valid {@link UpdateStrategyInstanceInput}
      *
      * Needs:
-     * - If name is given, must be non empty and match `/^[a-zA-Z0-9+/\-_= ]+$/`
+     * - If name is given, must be non empty
      * - If instanceConfig is given it must be an object
      * - If any of the flags is given it must be a valid boolean
      *
@@ -72,14 +72,6 @@ export class UpdateStrategyInstanceInput {
                 throw new HttpException("If name is given it must be a non empty string", HttpStatus.BAD_REQUEST);
             }
         }
-        if (input.name != undefined) {
-            if (input.name.match(/[^a-zA-Z0-9+/\-_= ]/g)) {
-                throw new HttpException(
-                    "Name of strategy instance may only contain alphanumeric characters, -, _, +, /, = and space",
-                    HttpStatus.BAD_REQUEST,
-                );
-            }
-        }
         if (input.instanceConfig != undefined && typeof input.instanceConfig != "object") {
             throw new HttpException("If instance config is given, it must be given as object", HttpStatus.BAD_REQUEST);
         }

backend/src/api-login/strategy/strategy-instances.controller.ts

@@ -81,6 +81,8 @@ export class StrategyInstancesController {
      * @returns The strategy instance with the given id (and type)
      */
     @Get(["strategyInstance/:id", "strategy/:type/instance/:id"])
+    @UseGuards(CheckAccessTokenGuard)
+    @NeedsAdmin()
     @ApiOperation({ summary: "Get one strategy instance" })
     @ApiParam({
         name: "id",
@@ -113,13 +115,6 @@ export class StrategyInstancesController {
             );
         }
         const strategy = this.strategiesService.getStrategyByName(instance.type);
-        const nonRedirectUrls = strategy.needsRedirectFlow
-            ? {}
-            : {
-                  postLogin: `/strategy/oauth/${instance.id}/token/login`,
-                  postRegister: `/strategy/oauth/${instance.id}/token/register`,
-                  postRegisterSync: `/strategy/oauth/${instance.id}/token/register-sync`,
-              };
         return {
             id: instance.id,
             name: instance.name,
@@ -128,12 +123,7 @@ export class StrategyInstancesController {
             isSelfRegisterActive: instance.isSelfRegisterActive,
             isSyncActive: instance.isSyncActive,
             doesImplicitRegister: instance.doesImplicitRegister,
-            urls: {
-                ...nonRedirectUrls,
-                redirectLogin: `/strategy/oauth/${instance.id}/authorize/login`,
-                redirectRegister: `/strategy/oauth/${instance.id}/authorize/register`,
-                redirectRegisterSync: `/strategy/oauth/${instance.id}/authorize/register-sync`,
-            },
+            instanceConfig: strategy.getCensoredInstanceConfig(instance),
         };
     }
 

backend/src/strategies/Strategy.ts

@@ -6,6 +6,7 @@ import { StrategiesService } from "src/model/services/strategies.service";
 import { StrategyInstanceService } from "src/model/services/strategy-instance.service";
 import { AuthResult, AuthStateServerData } from "./AuthResult";
 import { OAuthAuthorizeServerState } from "src/api-oauth/OAuthAuthorizeServerState";
+import { Schema } from "jtd";
 
 export interface StrategyVariable {
     name: string;
@@ -66,13 +67,13 @@ export abstract class Strategy {
         return instanceConfig;
     }
 
-    private updateCapabilityFlags(patrentValue: boolean, useDefault: boolean, inputValue?: boolean | null): boolean {
+    private updateCapabilityFlags(parentValue: boolean, useDefault: boolean, inputValue?: boolean | null): boolean {
         if (inputValue == null || inputValue == undefined) {
             if (useDefault) {
-                return patrentValue;
+                return parentValue;
             }
         } else if (typeof inputValue == "boolean") {
-            return patrentValue && inputValue;
+            return parentValue && inputValue;
         } else {
             throw new Error("Input value must be boolean");
         }
@@ -109,7 +110,7 @@ export abstract class Strategy {
         );
         instance.isSyncActive = this.updateCapabilityFlags(this.canSync, createNew, input.isSyncActive);
         if (createNew || input.name !== undefined) {
-            instance.name = input.name?.replace(/[^a-zA-Z0-9+/\-_= ]/g, "") ?? null;
+            instance.name = input.name ?? null;
         }
         if (createNew || input.instanceConfig) {
             instance.instanceConfig = resultingInstanceConfig ?? {};
@@ -133,9 +134,11 @@ export abstract class Strategy {
         });
     }
 
-    get acceptsVariables(): {
-        [variableName: string]: StrategyVariable;
-    } {
+    get acceptsVariables(): Record<string, StrategyVariable> {
+        return {};
+    }
+
+    get instanceConfigSchema(): Record<string, Schema> {
         return {};
     }
 
@@ -250,13 +253,29 @@ export abstract class Strategy {
         return {};
     }
 
+    /**
+     * Returns the instance config of the strategy instance, but with sensitive data censored.
+     * 
+     * **WARNING**: The result of this function WILL be exposed to the user.
+     * 
+     * @param instance The strategy instance for which to get the censored instance config
+     * @returns The censored instance config
+     */
+    getCensoredInstanceConfig(instance: StrategyInstance): object {
+        return {};
+    }
+
     abstract performAuth(
         strategyInstance: StrategyInstance,
         state: (AuthStateServerData & OAuthAuthorizeServerState) | undefined,
         req: any,
         res: any,
     ): Promise<PerformAuthResult>;
 
+    protected callbackUrlForStrategyInstance(strategyInstance: StrategyInstance): string {
+        return new URL(`/api/internal/auth/callback/${strategyInstance.id}`, process.env.GROPIUS_LOGIN_SERVICE_ENDPOINT).toString();
+    }
+
     toJSON() {
         return {
             typeName: this.typeName,
@@ -265,6 +284,7 @@ export abstract class Strategy {
             needsRedirectFlow: this.needsRedirectFlow,
             allowsImplicitSignup: this.allowsImplicitSignup,
             acceptsVariables: this.acceptsVariables,
+            instanceConfigSchema: this.instanceConfigSchema,
         };
     }
 }

backend/src/strategies/github/github.service.ts

@@ -12,6 +12,7 @@ import { UserLoginData } from "src/model/postgres/UserLoginData.entity";
 import { ActiveLoginService } from "src/model/services/active-login.service";
 import { checkType } from "../utils";
 import { OAuthAuthorizeServerState } from "src/api-oauth/OAuthAuthorizeServerState";
+import { Schema } from "jtd";
 
 @Injectable()
 export class GithubStrategyService extends StrategyUsingPassport {
@@ -27,6 +28,22 @@ export class GithubStrategyService extends StrategyUsingPassport {
         super("github", strategyInstanceService, strategiesService, stateJwtService, true, true, true, true);
     }
 
+    override get instanceConfigSchema(): Record<string, Schema> {
+        return {
+            imsTemplatedFieldsFilter: {
+                properties: {
+                    "graphql-url": { type: "string" },
+                },
+                nullable: true,
+            },
+            authorizationUrl: { type: "string", nullable: true },
+            tokenUrl: { type: "string", nullable: true },
+            userProfileUrl: { type: "string", nullable: true },
+            clientId: { type: "string", nullable: true },
+            clientSecret: { type: "string", nullable: true },
+        };
+    }
+
     /**
      * Chechs the given config is valid for a github (or github enterprise)
      *
@@ -88,7 +105,6 @@ export class GithubStrategyService extends StrategyUsingPassport {
                 true,
                 process.env.GROPIUS_OAUTH_CLIENT_SECRET,
             );
-            resultingConfig["callbackUrl"] = checkType(instanceConfig, "callbackUrl", "string", true);
         } catch (err) {
             throw new Error("Instance config for github instance invalid: " + err.message);
         }
@@ -196,7 +212,7 @@ export class GithubStrategyService extends StrategyUsingPassport {
                 userProfileURL: strategyInstance.instanceConfig["userProfileUrl"],
                 clientID: strategyInstance.instanceConfig["clientId"],
                 clientSecret: strategyInstance.instanceConfig["clientSecret"],
-                callbackURL: strategyInstance.instanceConfig["callbackUrl"],
+                callbackURL: this.callbackUrlForStrategyInstance(strategyInstance),
                 store: {
                     store: (req, state, meta, callback) => callback(null, state),
                     verify: (req, providedState, callback) => callback(null, true, providedState),
@@ -207,6 +223,17 @@ export class GithubStrategyService extends StrategyUsingPassport {
     }
 
     override async getLoginDataDescription(loginData: UserLoginData): Promise<string> {
-        return loginData.data?.username
+        return loginData.data?.username;
+    }
+
+    override getCensoredInstanceConfig(instance: StrategyInstance): object {
+        return {
+            imsTemplatedFieldsFilter: instance.instanceConfig["imsTemplatedFieldsFilter"],
+            authorizationUrl: instance.instanceConfig["authorizationUrl"],
+            tokenUrl: instance.instanceConfig["tokenUrl"],
+            userProfileUrl: instance.instanceConfig["userProfileUrl"],
+            clientId: instance.instanceConfig["clientId"],
+            clientSecret: "**********",
+        };
     }
 }