From 24534284668c6e3d4a60e19d894e9e6ff04a9eff Mon Sep 17 00:00:00 2001 From: bodymindarts Date: Wed, 15 Nov 2023 21:07:13 +0100 Subject: [PATCH] refactor: use scope for read only api keys --- core/api-keys/src/lib.rs | 1 + core/api-keys/src/scope.rs | 14 +++++ core/api-keys/src/server/mod.rs | 14 +++-- .../dev/apollo-federation/supergraph.graphql | 2 - core/api/src/domain/authorization/index.ts | 5 +- core/api/src/servers/graphql-main-server.ts | 10 +--- core/api/src/servers/index.files.d.ts | 1 - core/api/src/servers/middlewares/scope.ts | 60 +++++++++---------- core/api/src/servers/middlewares/session.ts | 4 +- dev/config/ory/oathkeeper_rules.yaml | 2 +- 10 files changed, 59 insertions(+), 54 deletions(-) create mode 100644 core/api-keys/src/scope.rs diff --git a/core/api-keys/src/lib.rs b/core/api-keys/src/lib.rs index f7745aa7b30..6208624fede 100644 --- a/core/api-keys/src/lib.rs +++ b/core/api-keys/src/lib.rs @@ -6,4 +6,5 @@ pub mod cli; mod entity; pub mod graphql; pub mod identity; +pub mod scope; pub mod server; diff --git a/core/api-keys/src/scope.rs b/core/api-keys/src/scope.rs new file mode 100644 index 00000000000..77bdab207cf --- /dev/null +++ b/core/api-keys/src/scope.rs @@ -0,0 +1,14 @@ +pub const READ_SCOPE: &str = "read"; +pub const WRITE_SCOPE: &str = "write"; + +pub fn read_only_scope() -> String { + format!("{READ_SCOPE}") +} + +pub fn read_write_scope() -> String { + format!("{READ_SCOPE} {WRITE_SCOPE}") +} + +pub fn is_read_only(scope: &String) -> bool { + !(scope.as_str().split(" ").any(|s| s == WRITE_SCOPE) || scope.is_empty()) +} diff --git a/core/api-keys/src/server/mod.rs b/core/api-keys/src/server/mod.rs index 376c6754ded..e93bfeefb32 100644 --- a/core/api-keys/src/server/mod.rs +++ b/core/api-keys/src/server/mod.rs @@ -21,7 +21,7 @@ pub struct JwtClaims { sub: String, exp: u64, #[serde(default)] - read_only: String, + scope: String, } pub async fn run_server(config: ServerConfig, api_keys_app: ApiKeysApp) -> anyhow::Result<()> { @@ -59,7 +59,7 @@ pub async fn run_server(config: ServerConfig, api_keys_app: ApiKeysApp) -> anyho #[derive(Debug, Serialize)] struct CheckResponse { sub: String, - read_only: bool, + scope: String, } async fn check_handler( @@ -68,7 +68,12 @@ async fn check_handler( ) -> Result, ApplicationError> { let key = headers.get(header).ok_or(ApplicationError::MissingApiKey)?; let (sub, read_only) = app.lookup_authenticated_subject(key.to_str()?).await?; - Ok(Json(CheckResponse { sub, read_only })) + let scope = if read_only { + crate::scope::read_only_scope() + } else { + crate::scope::read_write_scope() + }; + Ok(Json(CheckResponse { sub, scope })) } pub async fn graphql_handler( @@ -77,10 +82,11 @@ pub async fn graphql_handler( req: GraphQLRequest, ) -> GraphQLResponse { let req = req.into_inner(); + let read_only = crate::scope::is_read_only(&jwt_claims.scope); schema .execute(req.data(graphql::AuthSubject { id: jwt_claims.sub, - read_only: jwt_claims.read_only.eq("true"), + read_only, })) .await .into() diff --git a/core/api/dev/apollo-federation/supergraph.graphql b/core/api/dev/apollo-federation/supergraph.graphql index b0c019e4eb5..861c96d64be 100644 --- a/core/api/dev/apollo-federation/supergraph.graphql +++ b/core/api/dev/apollo-federation/supergraph.graphql @@ -180,7 +180,6 @@ type ApiKey expired: Boolean! lastUsedAt: Timestamp expiresAt: Timestamp! - readOnly: Boolean! } input ApiKeyCreateInput @@ -188,7 +187,6 @@ input ApiKeyCreateInput { name: String! expireInDays: Int - readOnly: Boolean! = false } type ApiKeyCreatePayload diff --git a/core/api/src/domain/authorization/index.ts b/core/api/src/domain/authorization/index.ts index 6d96a519aa8..aca3be2d505 100644 --- a/core/api/src/domain/authorization/index.ts +++ b/core/api/src/domain/authorization/index.ts @@ -1,6 +1,5 @@ export const ScopesOauth2 = { - TransactionsRead: "transactions:read", - Offline: "offline", - PaymentsSend: "payments:send", + Read: "read", + Write: "write", } as const export default ScopesOauth2 diff --git a/core/api/src/servers/graphql-main-server.ts b/core/api/src/servers/graphql-main-server.ts index 4bcd1ddcc3d..ad4b881f64c 100644 --- a/core/api/src/servers/graphql-main-server.ts +++ b/core/api/src/servers/graphql-main-server.ts @@ -43,14 +43,6 @@ export const isAuthenticated = rule({ cache: "contextual" })(( return "domainAccount" in ctx && !!ctx.domainAccount }) -export const isAuthenticatedForMutation = rule({ cache: "contextual" })(( - parent, - args, - ctx: GraphQLPublicContext, -) => { - return "domainAccount" in ctx && !!ctx.domainAccount && !ctx.readOnly -}) - const setGqlContext = async ( req: Request, res: Response, @@ -112,7 +104,7 @@ export async function startApolloServerForCoreSchema() { ...mutationFields.authed.atAccountLevel, ...mutationFields.authed.atWalletLevel, })) { - authedMutationFields[key] = isAuthenticatedForMutation + authedMutationFields[key] = isAuthenticated } const permissions = shield( diff --git a/core/api/src/servers/index.files.d.ts b/core/api/src/servers/index.files.d.ts index 957d77cebe2..add09bc539c 100644 --- a/core/api/src/servers/index.files.d.ts +++ b/core/api/src/servers/index.files.d.ts @@ -11,7 +11,6 @@ type GraphQLPublicContext = { loaders: Loaders ip: IpAddress | undefined sessionId: SessionId | undefined - readOnly: boolean } type GraphQLPublicContextAuth = GraphQLPublicContext & { diff --git a/core/api/src/servers/middlewares/scope.ts b/core/api/src/servers/middlewares/scope.ts index 9e4ea4c1de0..d6d8fbdb5c7 100644 --- a/core/api/src/servers/middlewares/scope.ts +++ b/core/api/src/servers/middlewares/scope.ts @@ -3,9 +3,9 @@ import { GraphQLResolveInfo, GraphQLFieldResolver } from "graphql" import ScopesOauth2 from "@/domain/authorization" import { AuthorizationError } from "@/domain/errors" import { mapError } from "@/graphql/error-map" -import { mutationFields } from "@/graphql/public" +import { mutationFields, queryFields } from "@/graphql/public" -const readTransactionsAuthorize = async ( +const readAuthorize = async ( resolve: GraphQLFieldResolver, parent: unknown, args: unknown, @@ -13,27 +13,21 @@ const readTransactionsAuthorize = async ( info: GraphQLResolveInfo, ) => { const scope = context.scope - const appId = context.appId - - // not a delegated token - if (appId === undefined || appId === "") { - return resolve(parent, args, context, info) - } + console.log("SCOPE ", scope) + // not a token with scope if (scope === undefined || scope.length === 0) { - return mapError( - new AuthorizationError("appId is defined but scope is undefined or empty"), - ) + return resolve(parent, args, context, info) } - if (scope.find((s) => s === ScopesOauth2.TransactionsRead) !== undefined) { + if (scope.find((s) => s === ScopesOauth2.Read) !== undefined) { return resolve(parent, args, context, info) } - return mapError(new AuthorizationError("not authorized to read transactions")) + return mapError(new AuthorizationError("not authorized to read data")) } -const paymentSendAuthorize = async ( +const writeAuthorize = async ( resolve: GraphQLFieldResolver, parent: unknown, args: unknown, @@ -41,26 +35,21 @@ const paymentSendAuthorize = async ( info: GraphQLResolveInfo, ) => { const scope = context.scope - const appId = context.appId - // not a delegated token - if (appId === undefined || appId === "") { + // not a token with scope + if (scope === undefined || scope.length === 0) { return resolve(parent, args, context, info) } - if (scope === undefined) { - return mapError(new AuthorizationError("appId is defined but scope is undefined")) - } - - if (scope.find((s) => s === ScopesOauth2.PaymentsSend) !== undefined) { + if (scope.find((s) => s === ScopesOauth2.Write) !== undefined) { return resolve(parent, args, context, info) } - return mapError(new AuthorizationError("not authorized to send payments")) + return mapError(new AuthorizationError("not authorized to execute mutations")) } // Placed here because 'GraphQLFieldResolver' not working from .d.ts file -type ValidateWalletIdFn = ( +type ValidateFn = ( resolve: GraphQLFieldResolver, parent: unknown, args: unknown, @@ -68,14 +57,23 @@ type ValidateWalletIdFn = ( info: GraphQLResolveInfo, ) => Promise -const walletIdMutationFields: { [key: string]: ValidateWalletIdFn } = {} -for (const key of Object.keys(mutationFields.authed.atWalletLevel)) { - walletIdMutationFields[key] = paymentSendAuthorize +const authedQueryFields: { [key: string]: ValidateFn } = {} +for (const key of Object.keys({ + ...queryFields.authed.atAccountLevel, + ...queryFields.authed.atWalletLevel, +})) { + authedQueryFields[key] = readAuthorize +} + +const authedMutationFields: { [key: string]: ValidateFn } = {} +for (const key of Object.keys({ + ...mutationFields.authed.atAccountLevel, + ...mutationFields.authed.atWalletLevel, +})) { + authedMutationFields[key] = writeAuthorize } export const scopeMiddleware = { - Query: { - me: readTransactionsAuthorize, - }, - Mutation: walletIdMutationFields, + Query: authedQueryFields, + Mutation: authedMutationFields, } diff --git a/core/api/src/servers/middlewares/session.ts b/core/api/src/servers/middlewares/session.ts index f1b2cbd943d..70416bfc7d8 100644 --- a/core/api/src/servers/middlewares/session.ts +++ b/core/api/src/servers/middlewares/session.ts @@ -25,10 +25,9 @@ export const sessionPublicContext = async ({ const sessionId = tokenPayload?.session_id const expiresAt = tokenPayload?.expires_at - const scope = tokenPayload?.scope?.split(" ") ?? [] + const scope = (tokenPayload?.scope?.split(" ") ?? []).filter((element: string) => element !== ''); const sub = tokenPayload?.sub const appId = tokenPayload?.client_id - const readOnly = tokenPayload?.read_only ?? false // note: value should match (ie: "anon") if not an accountId // settings from dev/ory/oathkeeper.yml/authenticator/anonymous/config/subjet @@ -86,6 +85,5 @@ export const sessionPublicContext = async ({ sessionId, scope, appId, - readOnly, } } diff --git a/dev/config/ory/oathkeeper_rules.yaml b/dev/config/ory/oathkeeper_rules.yaml index 5facf89fecc..3832f4a5144 100644 --- a/dev/config/ory/oathkeeper_rules.yaml +++ b/dev/config/ory/oathkeeper_rules.yaml @@ -87,7 +87,7 @@ mutators: - handler: id_token config: #! TODO: add aud: {"aud": ["https://api/graphql"] } - claims: '{"sub": "{{ print .Subject }}", "session_id": "{{ print .Extra.id }}", "expires_at": "{{ print .Extra.expires_at }}", "scope": "{{ print .Extra.scope }}", "client_id": "{{ print .Extra.client_id }}", "read_only": "{{ print .Extra.read_only }}" }' + claims: '{"sub": "{{ print .Subject }}", "session_id": "{{ print .Extra.id }}", "expires_at": "{{ print .Extra.expires_at }}", "scope": "{{ print .Extra.scope }}", "client_id": "{{ print .Extra.client_id }}"}' - id: admin-backend upstream: