Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(frontend, mock-ase): secure admin API with Hydra #2466

Closed
wants to merge 29 commits into from

Conversation

njlie
Copy link
Contributor

@njlie njlie commented Mar 1, 2024

Changes proposed in this pull request

  • Adds a Hydra image to the list of docker containers that are spun up
  • Adds a middleware to the admin API in backend that verifies the token with a request to the Hydra container.
    • Returns 401 if invalid token, continues to next() if valid
  • Adds a boilerplate login/consent screen to the Admin UI, to be replaced with ORY Kratos or something more sophisticated with design auth mechanism for admin UI #2200
  • Implements a basic Hydra authorization flow with the login & consent screens
    • Captures the authorization from this flow in a new session storage instance in the Remix app
  • Re-initializes the Admin UI Apollo Client each time it makes a request, so that it may pass in an auth token once one has been acquired
  • Uses updated error handling to handle & communicate Unauthorized responses from the Apollo client

Context

Fixes #2218 and #2497.
Adds a Hydra container, a basic authorization flow for it, and a middleware in the API to check for a valid authorization token for a given API request.

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. type: source Changes business logic pkg: mock-ase labels Mar 1, 2024
@njlie njlie changed the title feat: playing around with basic auth flow feat(frontend, mock-ase): secure admin API with Hydra Mar 1, 2024
@njlie njlie force-pushed the nl/2218/admin-graphql-api-security branch from 9a9a215 to ef7584f Compare March 13, 2024 22:14
@github-actions github-actions bot added pkg: auth Changes in the GNAP auth package. type: localenv Local playground pkg: documentation Changes in the documentation package. type: documentation (archived) Improvements or additions to documentation labels Mar 13, 2024
@njlie njlie changed the base branch from updated/1518/security-for-admin-ui to main March 13, 2024 22:18
@github-actions github-actions bot removed pkg: auth Changes in the GNAP auth package. type: localenv Local playground pkg: documentation Changes in the documentation package. type: documentation (archived) Improvements or additions to documentation labels Mar 13, 2024
Copy link

netlify bot commented Mar 13, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 44e4c95
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/65fc5cfe55ffff0008067085

@@ -284,13 +284,15 @@ export async function createWalletAddress(
})
}

export async function createWalletAddressKey({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just formatting in this file

export async function setupFromSeed(config: Config): Promise<void> {
const assets: Record<string, Asset> = {}
for (const { code, scale, liquidity, liquidityThreshold } of config.seed
.assets) {
const { asset } = await createAsset(code, scale, liquidityThreshold)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just formatting in this file as well

@@ -24,251 +24,300 @@ import type {
WithdrawAssetLiquidity,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This, and other files with the path /frontend/app/lib/api/*.server.ts, will have messy diffs but all follow the same change: the query is now nested inside of a try-catch block, and the catch block will catch the error, and if it is an Unauthenticated Apollo Error, it will throw an error formatted for Remix instead.

@@ -21,195 +21,238 @@ import {
type GetOutgoingPaymentVariables,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -22,202 +22,246 @@ import type {
WithdrawPeerLiquidity,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1,5 +1,5 @@
import { gql } from '@apollo/client'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1,51 +1,60 @@
import { gql } from '@apollo/client'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -16,8 +13,8 @@ BigInt.prototype.toJSON = function (this: bigint) {
return this.toString()
}

if (!global.__apolloClient) {
global.__apolloClient = new ApolloClient({
export function getApolloClient(token?: string) {
Copy link
Contributor Author

@njlie njlie Mar 13, 2024

Choose a reason for hiding this comment

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

We need to re-instantiate the Apollo Client each time a request is made now, so that it is able to pick up whether or not an authorization token is present. Normal stuff for React apps, in those cases it just happens inside of a React element whenever the app state changes.

@@ -4,6 +4,7 @@ import { v4 } from 'uuid'
import { LiquidityDialog } from '~/components/LiquidityDialog'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file, and other files in /app/routes/*.tsx all generally involve modifying the Remix loader and/or action to retrieve the API auth token from the session and passing it to whatever API call it's making.

@@ -18,19 +18,25 @@ import { Button, ErrorPanel, Input } from '~/components/ui'
import { FeeType } from '~/generated/graphql'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -4,6 +4,7 @@ import { v4 } from 'uuid'
import { LiquidityDialog } from '~/components/LiquidityDialog'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1,5 +1,6 @@
import { json, type LoaderFunctionArgs } from '@remix-run/node'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -4,6 +4,7 @@ import { PageHeader } from '~/components'
import { Button, Table } from '~/components/ui'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -4,6 +4,7 @@ import { PageHeader } from '~/components'
import { Button, ErrorPanel, Input } from '~/components/ui'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@njlie njlie force-pushed the nl/2218/admin-graphql-api-security branch from 7da3d34 to 7ee7d1c Compare March 18, 2024 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. pkg: mock-ase type: source Changes business logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design auth mechanism for admin graphql API
3 participants