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

fix: implement admin permissions #429

Draft
wants to merge 3 commits into
base: 4.0
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 53 additions & 2 deletions helm/api-platform/keycloak/config/realm-demo.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,61 @@
],
"clientScopes": [
{
"id": "45ece602-0403-472f-bec5-0b1ed8d3c4ec",
"name": "roles",
"description": "OpenID Connect scope for add user roles to the access token",
"protocol": "openid-connect",
"attributes": {
"include.in.token.scope": "true"
}
"include.in.token.scope": "true",
"display.on.consent.screen": "true",
"gui.order": "",
"consent.screen.text": "${rolesScopeConsentText}"
},
"protocolMappers": [
{
"id": "66670b4f-adc1-430b-bf4d-ac81cd03fb16",
"name": "client roles",
"protocol": "openid-connect",
"protocolMapper": "oidc-usermodel-client-role-mapper",
"consentRequired": false,
"config": {
"user.attribute": "foo",
"introspection.token.claim": "true",
"userinfo.token.claim": "true",
"access.token.claim": "true",
"claim.name": "resource_access.${client_id}.roles",
"jsonType.label": "String",
"multivalued": "true"
}
},
{
"id": "ac84d603-553b-487f-bc11-eb809a10df1a",
"name": "realm roles",
"protocol": "openid-connect",
"protocolMapper": "oidc-usermodel-realm-role-mapper",
"consentRequired": false,
"config": {
"user.attribute": "foo",
"introspection.token.claim": "true",
"userinfo.token.claim": "true",
"access.token.claim": "true",
"claim.name": "realm_access.roles",
"jsonType.label": "String",
"multivalued": "true"
}
},
{
"id": "a498765c-7294-481b-8143-318b06377834",
"name": "audience resolve",
"protocol": "openid-connect",
"protocolMapper": "oidc-audience-resolve-mapper",
"consentRequired": false,
"config": {
"introspection.token.claim": "true",
"access.token.claim": "true"
}
}
]
}
],
"clients": [
Expand Down
2 changes: 1 addition & 1 deletion pwa/app/auth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const { handlers: { GET, POST }, auth } = NextAuth({
// https://authjs.dev/guides/basics/refresh-token-rotation#jwt-strategy
params: {
access_type: "offline",
scope: "openid profile email",
scope: "openid profile email roles",
prompt: "consent",
},
},
Expand Down
8 changes: 7 additions & 1 deletion pwa/components/admin/Admin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import Head from "next/head";
import { useContext, useRef, useState } from "react";
import { type DataProvider, localStorageStore } from "react-admin";
import { type DataProvider, localStorageStore, usePermissions } from "react-admin";
import { signIn, useSession } from "next-auth/react";
import SyncLoader from "react-spinners/SyncLoader";
import {
Expand All @@ -22,6 +22,7 @@ import { ENTRYPOINT } from "../../config/entrypoint";
import bookResourceProps from "./book";
import reviewResourceProps from "./review";
import i18nProvider from "./i18nProvider";
import Logout from "./layout/Logout";

const apiDocumentationParser = (session: Session) => async () => {
try {
Expand Down Expand Up @@ -116,6 +117,7 @@ const AdminWithContext = ({ session }: { session: Session }) => {
const AdminWithOIDC = () => {
// Can't use next-auth/middleware because of https://github.com/nextauthjs/next-auth/discussions/7488
const { data: session, status } = useSession();
const { permissions } = usePermissions();
Copy link
Contributor

Choose a reason for hiding this comment

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

usePermissions can only be called by children of react-admin's <Admin> component (otherwise there is no AuthProviderContext and QueryClientContext available). That's why you have an error.

The proper fix would be to call the usePermissions in BookList and other admin-only components, and redirect to a /forbidden CustomRoute if the permissions don't match.


if (status === "loading") {
return <SyncLoader size={8} color="#46B6BF" />;
Expand All @@ -128,6 +130,10 @@ const AdminWithOIDC = () => {
return;
}

if (permissions !== 'admin') {
return <p id="forbidden">You are not allowed to access this page.</p>;
}

// @ts-ignore
return <AdminWithContext session={session} />;
};
Expand Down
18 changes: 17 additions & 1 deletion pwa/components/admin/authProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,23 @@ const authProvider: AuthProvider = {

return Promise.resolve();
},
getPermissions: () => Promise.resolve(),
getPermissions: async () => {
vincentchalamon marked this conversation as resolved.
Show resolved Hide resolved
const session = getSession();
const response = await fetch(`${NEXT_PUBLIC_OIDC_SERVER_URL}/protocol/openid-connect/userinfo`, {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
// @ts-ignore
Authorization: `Bearer ${session?.accessToken}`,
},
});
const token = await response.json();

if (!!token?.realm_access?.roles) {
return Promise.resolve(token.realm_access.roles);
}

return Promise.reject();
},
getIdentity: async () => {
const session = getSession();

Expand Down
21 changes: 18 additions & 3 deletions pwa/tests/admin/User.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { expect, test } from "./test";

test.describe("User authentication", () => {
test.beforeEach(async ({ bookPage }) => {
test("I can sign out of Admin @login", async ({ bookPage, page }) => {
await bookPage.gotoList();
});

test("I can sign out of Admin @login", async ({ userPage, page }) => {
await page.getByLabel("Profile").click();
await page.getByRole("menu").getByText("Logout").waitFor({ state: "visible" });
await page.getByRole("menu").getByText("Logout").click();
Expand All @@ -21,4 +19,21 @@ test.describe("User authentication", () => {
await expect(page.locator("#kc-form-login")).toContainText("Login as user: [email protected]");
await expect(page.locator("#kc-form-login")).toContainText("Login as admin: [email protected]");
});

test("I see a permission denied error page if I don't have the right permissions @login", async ({ userPage, page }) => {
await userPage.goToAdminWithInvalidUser();

await expect(page.locator("#forbidden")).toContainText("You are not allowed to access this page.");
await page.getByLabel("Profile").click();
await page.getByRole("menu").getByText("Logout").waitFor({ state: "visible" });
await page.getByRole("menu").getByText("Logout").click();

await expect(page).toHaveURL(/\/$/);

// I should be logged out from Keycloak also
await page.goto("/admin");
await page.waitForURL(/\/oidc\/realms\/demo\/protocol\/openid-connect\/auth/);
// @ts-ignore assert declared on test.ts
await expect(page).toBeOnLoginPage();
});
});
20 changes: 20 additions & 0 deletions pwa/tests/admin/pages/UserPage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,24 @@
import { AbstractPage } from "./AbstractPage";

export class UserPage extends AbstractPage {
public async goToAdminWithInvalidUser() {
await this.registerMock();

await this.page.goto("/admin");
await this.loginWithPublicUser();
await this.page.waitForURL(/\/admin#\/admin/);

return this.page;
}

public async loginWithPublicUser() {
await this.page.getByLabel("Email").fill("[email protected]");
await this.page.getByLabel("Password").fill("Pa55w0rd");
await this.page.getByRole("button", { name: "Sign In" }).click();
if (await this.page.getByRole("button", { name: "Sign in with Keycloak" }).count()) {
await this.page.getByRole("button", { name: "Sign in with Keycloak" }).click();
}

return this.page;
}
}
Loading