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
14 changes: 13 additions & 1 deletion web/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,16 @@ CHATGPT_MODEL_NAME=gpt-4o-2024-11-20
MISTRAL_LLM_API_KEY=ghi
MISTRAL_MODEL_NAME=open-mixtral-8x7b
ALBERT_LLM_API_KEY=jkl
ALBERT_MODEL_NAME=meta-llama/Meta-Llama-3.1-70B-Instruct
ALBERT_MODEL_NAME=meta-llama/Meta-Llama-3.1-70B-Instruct

# Generate secret: openssl rand -base64 32
NEXTAUTH_SECRET=todo
NEXTAUTH_URL=http://localhost:3000

# Use "integration" for testing, "production" for prod
PROCONNECT_ENV=integration
NEXT_PUBLIC_PROCONNECT_ENV=integration
Copy link
Collaborator

Choose a reason for hiding this comment

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

[CRITICAL] Avoid exposing environment selection to client-side

Why it matters:

  • NEXT_PUBLIC_* variables are embedded in client-side JavaScript bundles
  • Exposing environment configuration creates potential security risks
  • Attackers could identify integration vs production endpoints
  • This value should only be used server-side

Proposed change:
Remove NEXT_PUBLIC_PROCONNECT_ENV and use only the server-side PROCONNECT_ENV. Update proconnect-logout.ts to get the environment from an API endpoint or server-side prop instead of reading it directly from process.env.NEXT_PUBLIC_PROCONNECT_ENV.


# Get these from ProConnect portal
PROCONNECT_CLIENT_ID=
PROCONNECT_CLIENT_SECRET=
3 changes: 2 additions & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"debounce-promise": "^3.1.2",
"downshift": "^9.0.9",
"next": "15.3.3",
"next-auth": "^4.24.11",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"react-markdown": "^9.0.3",
Expand All @@ -42,4 +43,4 @@
"onFail": "error"
}
}
}
}
6 changes: 6 additions & 0 deletions web/src/app/access-denied/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { AccessDeniedContent } from "@/modules/auth/AccessDeniedContent";

export default function AccessDeniedPage() {
return <AccessDeniedContent />;
}

6 changes: 6 additions & 0 deletions web/src/app/api/auth/[...nextauth]/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import NextAuth from "next-auth";
import { authOptions } from "@/lib/auth/auth-options";

const handler = NextAuth(authOptions);

export { handler as GET, handler as POST };
31 changes: 0 additions & 31 deletions web/src/app/api/auth/route.ts

This file was deleted.

38 changes: 16 additions & 22 deletions web/src/hooks/use-auth.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,22 @@
"use client";

import { createContext, useContext, useState } from "react";
import { useSession } from "next-auth/react";

interface AuthContextType {
hasValidatedAuth: boolean;
setHasValidatedAuth: (value: boolean) => void;
}

const AuthContext = createContext<AuthContextType | null>(null);

export function AuthProvider({ children }: { children: React.ReactNode }) {
const [hasValidatedAuth, setHasValidatedAuth] = useState(false);
export function useAuth() {
const { data: session, status } = useSession();

return (
<AuthContext.Provider value={{ hasValidatedAuth, setHasValidatedAuth }}>
{children}
</AuthContext.Provider>
);
}
// A user is truly authenticated only if:
// 1. They have a valid session (status === "authenticated")
// 2. They are NOT marked as unauthorized
const isAuthenticated =
status === "authenticated" && !session?.unauthorized;

export function useAuth() {
const context = useContext(AuthContext);
if (!context) {
throw new Error("useAuth must be used within an AuthProvider");
}
return context;
return {
session,
status,
isAuthenticated,
isLoading: status === "loading",
user: session?.user,
isUnauthorized: session?.unauthorized === true,
};
}
137 changes: 137 additions & 0 deletions web/src/lib/auth/ProConnectProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import type { OAuthConfig, OAuthUserConfig } from "next-auth/providers/oauth";
import * as jose from "jose";

export interface ProConnectProfile {
sub: string;
email: string;
email_verified?: boolean;
given_name?: string;
usual_name?: string;
family_name?: string;
preferred_username?: string;
siret?: string;
organizational_unit?: string;
belonging_population?: string;
phone_number?: string;
job?: string;
}

export default function ProConnectProvider<P extends ProConnectProfile>(
options: OAuthUserConfig<P>
): OAuthConfig<P> {
// ProConnect/AgentConnect domains:
// This provider supports Internet environments (default)
// - Internet Integration: fca.integ01.dev-agentconnect.fr
// - Internet Production: auth.agentconnect.gouv.fr
// For RIE environments, set PROCONNECT_DOMAIN env var manually:
// - RIE Integration: fca.integ02.agentconnect.rie.gouv.fr
// - RIE Production: auth.agentconnect.rie.gouv.fr

const PROCONNECT_DOMAIN =
process.env.PROCONNECT_ENV === "production"
? "https://auth.agentconnect.gouv.fr/api/v2/"
: "https://fca.integ01.dev-agentconnect.fr/api/v2";

return {
id: "proconnect",
name: "ProConnect",
type: "oauth",
wellKnown: `${PROCONNECT_DOMAIN}/.well-known/openid-configuration`,
authorization: {
params: {
// ProConnect requires individual scopes for each claim
// See: https://partenaires.proconnect.gouv.fr/docs/fournisseur-service/scope-claims
scope: "openid email given_name usual_name uid siret",
acr_values: "eidas1", // Level of authentication required
},
},
idToken: true,
checks: ["pkce", "state"],
client: {
token_endpoint_auth_method: "client_secret_post",
},
// ProConnect returns userinfo as a JWT signed with HS256 (using client_secret)
// We need to manually fetch and verify it
// See: https://partenaires.proconnect.gouv.fr/docs/fournisseur-service/scope-claims
userinfo: {
url: `${PROCONNECT_DOMAIN}/userinfo`,
async request({ tokens }) {
const response = await fetch(`${PROCONNECT_DOMAIN}/userinfo`, {
headers: {
Authorization: `Bearer ${tokens.access_token}`,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

[IMPORTANT] Add error handling for userinfo request failures

Why it matters:

  • Network failures or ProConnect API issues will cause unhandled promise rejections
  • Users will see generic error pages instead of helpful authentication error messages
  • No retry logic or graceful degradation

Proposed change:
Wrap the userinfo request in try-catch and provide better error context:

async request({ tokens }) {
  try {
    const response = await fetch(`${PROCONNECT_DOMAIN}/userinfo`, {
      headers: {
        Authorization: `Bearer ${tokens.access_token}`,
      },
    });

    if (!response.ok) {
      throw new Error(`Userinfo request failed: ${response.status} ${response.statusText}`);
    }
    // ... rest of the code
  } catch (error) {
    console.error('ProConnect userinfo request failed:', error);
    throw new Error('Failed to fetch user information from ProConnect. Please try again.');
  }
}

});

if (!response.ok) {
throw new Error(`Userinfo request failed: ${response.status}`);
}

// ProConnect returns userinfo as a signed JWT (HS256, RS256, or ES256)
const jwt = await response.text();
const header = jose.decodeProtectedHeader(jwt);

if (header.alg === "HS256") {
// For HS256, verify with client_secret
const secret = new TextEncoder().encode(options.clientSecret);
const { payload } = await jose.jwtVerify(jwt, secret, {
algorithms: ["HS256"],
});
return payload as unknown as P;
} else if (header.alg === "RS256" || header.alg === "ES256") {
// For RS256/ES256, verify with JWKS
const jwksUrls = [
`${PROCONNECT_DOMAIN}/jwks`,
`${PROCONNECT_DOMAIN}/.well-known/jwks.json`,
];

for (const jwksUrl of jwksUrls) {
try {
const JWKS = jose.createRemoteJWKSet(new URL(jwksUrl));
const { payload } = await jose.jwtVerify(jwt, JWKS, {
algorithms: ["RS256", "ES256"],
});
return payload as unknown as P;
} catch {
// Try next URL
continue;
}
}

throw new Error(
`Could not verify ${header.alg} JWT with available JWKS endpoints`
);
Comment on lines +80 to +102
Copy link
Collaborator

Choose a reason for hiding this comment

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

[IMPORTANT] JWT verification error handling is incomplete

Why it matters:

  • If JWT verification fails, the error message is unclear
  • Multiple JWKS URLs are tried but errors are silently swallowed
  • Debugging authentication issues will be difficult

Proposed change:
Collect and report all verification errors:

const errors: Error[] = [];
for (const jwksUrl of jwksUrls) {
  try {
    const JWKS = jose.createRemoteJWKSet(new URL(jwksUrl));
    const { payload } = await jose.jwtVerify(jwt, JWKS, {
      algorithms: ["RS256", "ES256"],
    });
    return payload as unknown as P;
  } catch (error) {
    errors.push(error as Error);
    continue;
  }
}

throw new Error(
  `Could not verify ${header.alg} JWT with available JWKS endpoints. Errors: ${errors.map(e => e.message).join(', ')}`
);
Suggested change
} else if (header.alg === "RS256" || header.alg === "ES256") {
// For RS256/ES256, verify with JWKS
const jwksUrls = [
`${PROCONNECT_DOMAIN}/jwks`,
`${PROCONNECT_DOMAIN}/.well-known/jwks.json`,
];
for (const jwksUrl of jwksUrls) {
try {
const JWKS = jose.createRemoteJWKSet(new URL(jwksUrl));
const { payload } = await jose.jwtVerify(jwt, JWKS, {
algorithms: ["RS256", "ES256"],
});
return payload as unknown as P;
} catch {
// Try next URL
continue;
}
}
throw new Error(
`Could not verify ${header.alg} JWT with available JWKS endpoints`
);
} else if (header.alg === "RS256" || header.alg === "ES256") {
// For RS256/ES256, verify with JWKS
const jwksUrls = [
`${PROCONNECT_DOMAIN}/jwks`,
`${PROCONNECT_DOMAIN}/.well-known/jwks.json`,
];
const errors: Error[] = [];
for (const jwksUrl of jwksUrls) {
try {
const JWKS = jose.createRemoteJWKSet(new URL(jwksUrl));
const { payload } = await jose.jwtVerify(jwt, JWKS, {
algorithms: ["RS256", "ES256"],
});
return payload as unknown as P;
} catch (error) {
errors.push(error as Error);
continue;
}
}
throw new Error(
`Could not verify ${header.alg} JWT with available JWKS endpoints. Errors: ${errors.map(e => e.message).join(', ')}`
);

}

throw new Error(`Unsupported JWT algorithm: ${header.alg}`);
},
},
profile(profile) {
return {
id: profile.sub,
email: profile.email,
name: profile.given_name
? `${profile.given_name} ${
profile.usual_name || profile.family_name || ""
}`
: profile.preferred_username || profile.email,
image: null,
given_name: profile.given_name,
family_name: profile.family_name || profile.usual_name,
siret: profile.siret,
organizational_unit: profile.organizational_unit,
belonging_population: profile.belonging_population,
phone_number: profile.phone_number,
job: profile.job,
};
},
style: {
logo: "/proconnect-logo.svg",
logoDark: "/proconnect-logo.svg",
bg: "#fff",
text: "#000091",
bgDark: "#000091",
textDark: "#fff",
},
options,
};
}
91 changes: 91 additions & 0 deletions web/src/lib/auth/auth-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { NextAuthOptions } from "next-auth";
import ProConnectProvider, { ProConnectProfile } from "./ProConnectProvider";
import * as Sentry from "@sentry/nextjs";

// Allowed email domains for access control
const ALLOWED_EMAIL_DOMAINS = [
"pyrenees-atlantiques.gouv.fr",
"seine-maritime.gouv.fr",
"correze.gouv.fr",
"dreets.gouv.fr",
"travail.gouv.fr",
"fabrique.social.gouv.fr",
"sg.social.gouv.fr",
// Add beta.gouv.fr for local development
...(process.env.NODE_ENV === "development" ? ["beta.gouv.fr"] : []),
];

// Helper function to check if email domain is allowed
Copy link
Collaborator

Choose a reason for hiding this comment

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

[MINOR] Email domain comparison should be case-insensitive throughout

Why it matters:

  • Email addresses are case-insensitive per RFC 5321
  • Current code converts emailDomain to lowercase but compares against mixed-case ALLOWED_EMAIL_DOMAINS
  • Could cause false negatives if domains in the array have uppercase characters

Note: The comparison at line 26 does use .toLowerCase() on both sides, so this is already handled correctly. However, for clarity and to prevent future bugs, consider normalizing the ALLOWED_EMAIL_DOMAINS array itself to lowercase at definition time.

function isEmailDomainAllowed(email: string | null | undefined): boolean {
if (!email) return false;

const emailDomain = email.split("@")[1]?.toLowerCase();
if (!emailDomain) return false;

return ALLOWED_EMAIL_DOMAINS.some(
(domain) => emailDomain === domain.toLowerCase()
);
}

export const authOptions: NextAuthOptions = {
providers: [
ProConnectProvider({
clientId: process.env.PROCONNECT_CLIENT_ID!,
clientSecret: process.env.PROCONNECT_CLIENT_SECRET!,
}),
],
Comment on lines +30 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

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

CRITICAL: Missing environment variable validation. If PROCONNECT_CLIENT_ID or PROCONNECT_CLIENT_SECRET are not set, the app will crash with a cryptic error. This should fail fast with a clear error message during startup.

The non-null assertion operator (!) bypasses TypeScript's safety checks without runtime validation.

Suggested change
export const authOptions: NextAuthOptions = {
providers: [
ProConnectProvider({
clientId: process.env.PROCONNECT_CLIENT_ID!,
clientSecret: process.env.PROCONNECT_CLIENT_SECRET!,
}),
],
// Validate required environment variables
if (!process.env.PROCONNECT_CLIENT_ID) {
throw new Error('PROCONNECT_CLIENT_ID environment variable is required');
}
if (!process.env.PROCONNECT_CLIENT_SECRET) {
throw new Error('PROCONNECT_CLIENT_SECRET environment variable is required');
}
if (!process.env.NEXTAUTH_SECRET) {
throw new Error('NEXTAUTH_SECRET environment variable is required');
}
export const authOptions: NextAuthOptions = {
providers: [
ProConnectProvider({
clientId: process.env.PROCONNECT_CLIENT_ID,
clientSecret: process.env.PROCONNECT_CLIENT_SECRET,
}),
],

Comment on lines +30 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

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

[CRITICAL] Missing environment variable validation

Why it matters:

  • Application will crash at runtime if PROCONNECT_CLIENT_ID or PROCONNECT_CLIENT_SECRET are undefined
  • Error will occur during NextAuth initialization, making the entire app unusable
  • No clear error message for developers during setup

Proposed change:
Add validation at the top of the file with clear error messages:

if (!process.env.PROCONNECT_CLIENT_ID || !process.env.PROCONNECT_CLIENT_SECRET) {
  throw new Error(
    'Missing required ProConnect environment variables. Please set PROCONNECT_CLIENT_ID and PROCONNECT_CLIENT_SECRET in your .env file.'
  )
}
Suggested change
export const authOptions: NextAuthOptions = {
providers: [
ProConnectProvider({
clientId: process.env.PROCONNECT_CLIENT_ID!,
clientSecret: process.env.PROCONNECT_CLIENT_SECRET!,
}),
],
// Validate required environment variables
if (!process.env.PROCONNECT_CLIENT_ID || !process.env.PROCONNECT_CLIENT_SECRET) {
throw new Error(
'Missing required ProConnect environment variables. Please set PROCONNECT_CLIENT_ID and PROCONNECT_CLIENT_SECRET in your .env file.'
)
}
export const authOptions: NextAuthOptions = {
providers: [
ProConnectProvider({
clientId: process.env.PROCONNECT_CLIENT_ID,
clientSecret: process.env.PROCONNECT_CLIENT_SECRET,
}),
],

callbacks: {
async signIn() {
// Always return true to allow session creation with id_token
// This is needed for proper ProConnect logout (requires id_token_hint)
//
// Security: Authorization is enforced in multiple layers:
// 1. JWT callback marks unauthorized users (token.unauthorized = true)
// 2. useAuth() returns isAuthenticated = false for unauthorized users
// 3. AuthorizationCheck redirects unauthorized users to /access-denied
//
// This approach prevents the "user jail" problem while maintaining security
return true;
},
async jwt({ token, account, profile, user }) {
// Persist the OAuth tokens and profile after signin
if (account) {
token.accessToken = account.access_token;
token.idToken = account.id_token;
}
if (profile) {
token.profile = profile as ProConnectProfile;

// Check if the user's email domain is allowed
const email = profile.email || user?.email;
if (!isEmailDomainAllowed(email)) {
token.unauthorized = true;
}
}
return token;
},
async session({ session, token }) {
// Pass tokens and profile to the client session
session.accessToken = token.accessToken as string;
session.idToken = token.idToken as string;
session.profile = token.profile as ProConnectProfile | undefined;
session.unauthorized = token.unauthorized as boolean | undefined;
return session;
},
},
pages: {
signIn: "/",
error: "/access-denied",
},
events: {
async signIn({ user }) {
Sentry.setUser({
id: user.id,
email: user.email || undefined,
});
},
async signOut() {
Sentry.setUser(null);
},
},
};
39 changes: 39 additions & 0 deletions web/src/lib/auth/proconnect-logout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* ProConnect logout utilities
* Handles proper logout from both NextAuth and ProConnect (OpenID Connect provider)
*/

/**
* Get the ProConnect end_session_endpoint URL based on environment
*/
export function getProConnectLogoutUrl(): string {
const PROCONNECT_DOMAIN =
process.env.NEXT_PUBLIC_PROCONNECT_ENV === "production"
Copy link
Collaborator

Choose a reason for hiding this comment

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

[MINOR] Consider validating environment variable

Why it matters:

  • If NEXT_PUBLIC_PROCONNECT_ENV is undefined or has an unexpected value, the function silently defaults to integration
  • This could cause production apps to use integration endpoints

Proposed change:
Add validation:

export function getProConnectLogoutUrl(): string {
  const env = process.env.NEXT_PUBLIC_PROCONNECT_ENV;
  
  if (env && env !== 'production' && env !== 'integration') {
    console.warn(`Invalid NEXT_PUBLIC_PROCONNECT_ENV: ${env}, defaulting to integration`);
  }
  
  const PROCONNECT_DOMAIN =
    env === "production"
      ? "https://auth.agentconnect.gouv.fr/api/v2"
      : "https://fca.integ01.dev-agentconnect.fr/api/v2";

  return `${PROCONNECT_DOMAIN}/session/end`;
}

? "https://auth.agentconnect.gouv.fr/api/v2"
: "https://fca.integ01.dev-agentconnect.fr/api/v2";

// OpenID Connect standard logout endpoint
return `${PROCONNECT_DOMAIN}/session/end`;
}

/**
* Build the complete logout URL for ProConnect with required parameters
* @param idToken - The ID token from the current session (optional - per OIDC spec)
* @param postLogoutRedirectUri - Where to redirect after logout (must be registered in ProConnect)
*/
export function buildProConnectLogoutUrl(
idToken: string,
postLogoutRedirectUri: string
): string {
const logoutUrl = getProConnectLogoutUrl();
const params = new URLSearchParams({
post_logout_redirect_uri: postLogoutRedirectUri,
});

// Only add id_token_hint if we have one (it's optional per OIDC spec)
if (idToken) {
params.set("id_token_hint", idToken);
}

return `${logoutUrl}?${params.toString()}`;
}
Loading
Loading