Add Generic sso/OIDC #701
Add Generic sso/OIDC #701
Conversation
chore(env): missing var in .env.example chore: typo chore: add reviewed stuff fix: rebased stuff fix(authentik-oidc): redirect corect oidc provider
|
@DrummyFloyd is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a set of new OAuth configuration options and functionality across both backend and frontend. Environment variables related to OAuth have been added, and a new backend Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant LC as Login Component
participant FP as Frontend OAuth Component
participant BE as Backend OAuth Provider
participant OS as External OAuth Service
U->>LC: Click "Sign in"
LC->>FP: Render OAuth Provider component
FP->>BE: Request OAuth login link (/auth/oauth/GENERIC)
BE->>BE: Generate link using OauthProvider.generateLink()
BE-->>FP: Return OAuth URL
FP->>U: Redirect to External OAuth Service
U->>OS: Authenticate and authorize
OS-->>U: Redirect back with auth code
U->>BE: Submit auth code for token exchange
BE->>BE: Invoke getToken() and getUser() methods
BE-->>U: Return authenticated user info
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
apps/frontend/src/components/auth/login.tsx (1)
68-70: Consider simplifying conditional rendering logic.While the implementation works correctly, the nested ternary structure could be made more readable.
- {isGeneral && genericOauth ? ( - <OauthProvider /> - ) : !isGeneral ? ( - <GithubProvider /> - ) : ( + {!isGeneral ? ( + <GithubProvider /> + ) : isGeneral && genericOauth ? ( + <OauthProvider /> + ) : ( <div className="gap-[5px] flex flex-col"> <GoogleProvider /> {!!neynarClientId && <FarcasterProvider />}apps/frontend/src/components/auth/providers/oauth.provider.tsx (1)
31-38: Add loading state and improve accessibilityThe component lacks a loading state during the API request and could benefit from improved accessibility attributes.
+ const [loading, setLoading] = useState(false); const gotoLogin = useCallback(async () => { try { + setLoading(true); const response = await fetch('/auth/oauth/GENERIC'); // ... existing code } catch (error) { // ... existing error handling + } finally { + setLoading(false); } }, []); return ( <div onClick={gotoLogin} className={`cursor-pointer bg-white h-[44px] rounded-[4px] flex justify-center items-center text-customColor16 ${interClass} gap-[4px]`} + role="button" + aria-label={`Sign in with ${oauthDisplayName || 'OAuth'}`} + tabIndex={0} + onKeyDown={(e) => e.key === 'Enter' && gotoLogin()} > <div> <Image src={oauthLogoUrl || '/icons/generic-oauth.svg'} alt={`${oauthDisplayName || 'OAuth'} logo`} width={40} height={40} /> </div> - <div>Sign in with {oauthDisplayName || 'OAuth'}</div> + <div>{loading ? 'Connecting...' : `Sign in with ${oauthDisplayName || 'OAuth'}`}</div> </div> );.env.example (2)
92-102: Add documentation comments and fix inconsistent URL domainThe OAuth environment variables lack documentation comments explaining their purpose, unlike other sections in the file. Additionally, there's an inconsistency in the domain used for the userinfo URL.
+# OAuth settings for authentication providers like Authentik NEXT_PUBLIC_POSTIZ_OAUTH_DISPLAY_NAME="Authentik" NEXT_PUBLIC_POSTIZ_OAUTH_LOGO_URL="https://raw.githubusercontent.com/walkxcode/dashboard-icons/master/png/authentik.png" POSTIZ_GENERIC_OAUTH="false" POSTIZ_OAUTH_URL="https://auth.example.com" POSTIZ_OAUTH_AUTH_URL="https://auth.example.com/application/o/authorize" POSTIZ_OAUTH_TOKEN_URL="https://auth.example.com/application/o/token" -POSTIZ_OAUTH_USERINFO_URL="https://authentik.example.com/application/o/userinfo" +POSTIZ_OAUTH_USERINFO_URL="https://auth.example.com/application/o/userinfo" POSTIZ_OAUTH_CLIENT_ID="" POSTIZ_OAUTH_CLIENT_SECRET="" +# OAuth scopes to request (optional, defaults to "openid profile email") # POSTIZ_OAUTH_SCOPE="openid profile email" # default values
94-94: Fix boolean string representationThe
POSTIZ_GENERIC_OAUTHvalue should use lowercase "true" or "false" to align with JavaScript boolean string representation conventions.-POSTIZ_GENERIC_OAUTH="false" +POSTIZ_GENERIC_OAUTH="false" # Use "true" to enable generic OAuth authenticationlibraries/react-shared-libraries/src/helpers/variable.context.tsx (2)
8-10: Add JSDoc comments to document new interface propertiesThe new properties added to the interface lack documentation, which would help other developers understand their purpose.
interface VariableContextInterface { billingEnabled: boolean; isGeneral: boolean; + /** Whether to enable the generic OAuth provider (e.g., Authentik) */ genericOauth: boolean; + /** URL for the OAuth provider's logo */ oauthLogoUrl: string; + /** Display name for the OAuth provider */ oauthDisplayName: string; frontEndUrl: string; plontoKey: string;
13-13: Consider extending StorageProvider type for future expansionThe storage provider type is constrained to only 'local' or 'cloudflare', which might limit future extensibility.
- storageProvider: 'local' | 'cloudflare'; + storageProvider: 'local' | 'cloudflare' | string;Alternatively, you could define a dedicated type:
type StorageProviderType = 'local' | 'cloudflare' | string; interface VariableContextInterface { // ... storageProvider: StorageProviderType; // ... }apps/backend/src/services/auth/providers/oauth.provider.ts (4)
3-11: Remove unused property or verify its usage.
ThebaseUrlproperty is assigned but never read anywhere in the class. This unused property can cause confusion and reduce maintainability. Consider removing it if not truly needed.
12-49: Consolidate environment variable checks for clarity.
Although the constructor’s environment variable checks are thorough, you might simplify by gathering all missing variables into a single error message or by using a schema validation approach (e.g., a library or custom validator). This makes it easier to spot all missing variables at once and improves maintainability.
51-60: Allow customization of scope and redirect URI.
Currently, the method hardcodesscope: 'openid profile email'and usessettingsas the redirect route. It may be beneficial to make these configurable via environment variables or configuration files for more flexibility, especially if you need to handle different grant flows or future expansions.
62-85: Consider PKCE or additional security measures for OAuth.
The code flow here is functional, but to enhance security, particularly for public clients, you might incorporate a Proof Key for Code Exchange (PKCE) flow and/or other OAuth best practices such as nonce parameters. This helps guard against interception attacks and aligns closely with modern OAuth standards.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/public/icons/generic-oauth.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
.env.example(3 hunks)apps/backend/src/services/auth/providers/oauth.provider.ts(1 hunks)apps/backend/src/services/auth/providers/providers.factory.ts(2 hunks)apps/frontend/src/app/layout.tsx(1 hunks)apps/frontend/src/components/auth/login.tsx(3 hunks)apps/frontend/src/components/auth/providers/oauth.provider.tsx(1 hunks)apps/frontend/src/middleware.ts(1 hunks)libraries/nestjs-libraries/src/database/prisma/schema.prisma(2 hunks)libraries/react-shared-libraries/src/helpers/variable.context.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/backend/src/services/auth/providers/providers.factory.ts (1)
apps/backend/src/services/auth/providers/oauth.provider.ts (1)
OauthProvider(3-103)
apps/frontend/src/components/auth/providers/oauth.provider.tsx (2)
libraries/helpers/src/utils/custom.fetch.tsx (1)
useFetch(44-46)libraries/react-shared-libraries/src/helpers/variable.context.tsx (1)
useVariables(59-61)
🔇 Additional comments (8)
apps/backend/src/services/auth/providers/providers.factory.ts (2)
7-7: Good addition of the OauthProvider import.The import is correctly added, allowing the factory to access the OAuth provider implementation.
20-21: LGTM! Provider.GENERIC handler added appropriately.The switch case for the GENERIC provider is properly implemented, following the existing pattern in the factory. This enables the application to instantiate the OAuth provider for authentication.
libraries/nestjs-libraries/src/database/prisma/schema.prisma (2)
639-639: Good addition of GENERIC to Provider enum.The GENERIC provider type is correctly added to the Provider enum, which is necessary for the OAuth authentication functionality to work with the database schema.
652-652: Minor formatting improvement.Added a newline at the end of the APPROVED_SUBMIT_FOR_ORDER enum, which is a good practice for maintaining consistent formatting in the schema file.
apps/frontend/src/components/auth/login.tsx (2)
12-12: Good import of the OauthProvider component.The OauthProvider component is properly imported, enabling OAuth authentication in the login UI.
28-29: Correctly added genericOauth to useVariables destructuring.The genericOauth flag is now properly extracted from the variables context, enabling conditional rendering of the OAuth provider.
apps/frontend/src/app/layout.tsx (1)
42-44:✅ Verification successful
Good addition of OAuth configuration props.
The OAuth-related properties are correctly added to the VariableContextComponent:
- genericOauth flag is properly converted to boolean
- oauthLogoUrl and oauthDisplayName are passed through from environment variables
This enables the frontend to access OAuth configuration from the context.
Consider adding default values for oauthLogoUrl and oauthDisplayName to handle cases where the environment variables might not be set:
🏁 Script executed:
#!/bin/bash # Check if the corresponding environment variables exist in .env.example grep -E "NEXT_PUBLIC_POSTIZ_OAUTH_(LOGO_URL|DISPLAY_NAME)" .env.example || echo "Warning: OAuth environment variables not defined in .env.example"Length of output: 244
OAuth configuration props are implemented correctly.
- The changes in
apps/frontend/src/app/layout.tsxcorrectly handle the OAuth properties by convertinggenericOauthto a boolean and passingoauthLogoUrlandoauthDisplayNamefrom the environment variables.- The
.env.examplefile already defines default values forNEXT_PUBLIC_POSTIZ_OAUTH_DISPLAY_NAMEandNEXT_PUBLIC_POSTIZ_OAUTH_LOGO_URL, ensuring that default settings are available during initial setup.- While adding in-code fallback defaults could further safeguard against runtime issues in production, this may be optional given that the necessary defaults are maintained in
.env.example.apps/backend/src/services/auth/providers/oauth.provider.ts (1)
87-102: Validate email and subject claims in user info response.
Should the user info response omit or fail to return validsub, the code will silently fail with minimal context. Consider adding checks or fallbacks (e.g., default IDs, placeholders, or further error logging) to handle incomplete data gracefully.
| ? process.env.POSTIZ_GENERIC_OAUTH | ||
| ? 'generic' | ||
| : 'github' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety for environment variable check
The condition checks process.env.POSTIZ_GENERIC_OAUTH directly without type conversion, which may lead to unexpected behavior since environment variables are strings. A more robust approach would explicitly convert to boolean.
- ? process.env.POSTIZ_GENERIC_OAUTH
+ ? process.env.POSTIZ_GENERIC_OAUTH === 'true'
? 'generic'
: 'github'Additionally, consider renaming 'generic' to 'oauth' or 'authentik' to better reflect the PR's purpose of adding Authentik SSO support.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ? process.env.POSTIZ_GENERIC_OAUTH | |
| ? 'generic' | |
| : 'github' | |
| ? process.env.POSTIZ_GENERIC_OAUTH === 'true' | |
| ? 'generic' | |
| : 'github' |
| const gotoLogin = useCallback(async () => { | ||
| try { | ||
| const response = await fetch('/auth/oauth/GENERIC'); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Login link request failed with status ${response.status}` | ||
| ); | ||
| } | ||
| const link = await response.text(); | ||
| window.location.href = link; | ||
| } catch (error) { | ||
| console.error('Failed to get generic oauth login link:', error); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add user-facing error handling
The error handling currently only logs to the console, but there's no feedback mechanism for the user if the OAuth login link request fails.
Consider implementing a state variable to track errors and display a user-friendly message:
import { useCallback } from 'react';
+import { useState } from 'react';
import Image from 'next/image';
import { useFetch } from '@gitroom/helpers/utils/custom.fetch';
import interClass from '@gitroom/react/helpers/inter.font';
import { useVariables } from '@gitroom/react/helpers/variable.context';
export const OauthProvider = () => {
const fetch = useFetch();
const { oauthLogoUrl, oauthDisplayName } = useVariables();
+ const [error, setError] = useState<string | null>(null);
const gotoLogin = useCallback(async () => {
try {
+ setError(null);
const response = await fetch('/auth/oauth/GENERIC');
if (!response.ok) {
throw new Error(
`Login link request failed with status ${response.status}`
);
}
const link = await response.text();
window.location.href = link;
} catch (error) {
console.error('Failed to get generic oauth login link:', error);
+ setError('Failed to initiate login. Please try again later.');
}
}, []);Then display the error message in the UI:
return (
<div
onClick={gotoLogin}
className={`cursor-pointer bg-white h-[44px] rounded-[4px] flex justify-center items-center text-customColor16 ${interClass} gap-[4px]`}
>
<div>
<Image
src={oauthLogoUrl || '/icons/generic-oauth.svg'}
alt="genericOauth"
width={40}
height={40}
/>
</div>
<div>Sign in with {oauthDisplayName || 'OAuth'}</div>
+ {error && <div className="text-red-500 text-sm mt-2">{error}</div>}
</div>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const gotoLogin = useCallback(async () => { | |
| try { | |
| const response = await fetch('/auth/oauth/GENERIC'); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Login link request failed with status ${response.status}` | |
| ); | |
| } | |
| const link = await response.text(); | |
| window.location.href = link; | |
| } catch (error) { | |
| console.error('Failed to get generic oauth login link:', error); | |
| } | |
| }, []); | |
| import { useCallback } from 'react'; | |
| import { useState } from 'react'; | |
| import Image from 'next/image'; | |
| import { useFetch } from '@gitroom/helpers/utils/custom.fetch'; | |
| import interClass from '@gitroom/react/helpers/inter.font'; | |
| import { useVariables } from '@gitroom/react/helpers/variable.context'; | |
| export const OauthProvider = () => { | |
| const fetch = useFetch(); | |
| const { oauthLogoUrl, oauthDisplayName } = useVariables(); | |
| const [error, setError] = useState<string | null>(null); | |
| const gotoLogin = useCallback(async () => { | |
| try { | |
| setError(null); | |
| const response = await fetch('/auth/oauth/GENERIC'); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Login link request failed with status ${response.status}` | |
| ); | |
| } | |
| const link = await response.text(); | |
| window.location.href = link; | |
| } catch (error) { | |
| console.error('Failed to get generic oauth login link:', error); | |
| setError('Failed to initiate login. Please try again later.'); | |
| } | |
| }, []); | |
| return ( | |
| <div | |
| onClick={gotoLogin} | |
| className={`cursor-pointer bg-white h-[44px] rounded-[4px] flex justify-center items-center text-customColor16 ${interClass} gap-[4px]`} | |
| > | |
| <div> | |
| <Image | |
| src={oauthLogoUrl || '/icons/generic-oauth.svg'} | |
| alt="genericOauth" | |
| width={40} | |
| height={40} | |
| /> | |
| </div> | |
| <div>Sign in with {oauthDisplayName || 'OAuth'}</div> | |
| {error && <div className="text-red-500 text-sm mt-2">{error}</div>} | |
| </div> | |
| ); | |
| }; |
|
@nevo-david any chance this will be able to get merged in a version soon? |
bump |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/frontend/src/middleware.ts (1)
47-49: 🛠️ Refactor suggestionEnhance type safety for environment variable check.
The condition checks
process.env.POSTIZ_GENERIC_OAUTHdirectly without type conversion, which may lead to unexpected behavior since environment variables are strings. A more robust approach would explicitly convert to boolean.- ? process.env.POSTIZ_GENERIC_OAUTH + ? process.env.POSTIZ_GENERIC_OAUTH === 'true' ? 'generic' : 'github'apps/frontend/src/components/auth/providers/oauth.provider.tsx (1)
11-24: Add user-facing error handlingThe error handling currently only logs to the console, but there's no feedback mechanism for the user if the OAuth login link request fails.
Consider implementing a state variable to track errors and display a user-friendly message:
import { useCallback } from 'react'; +import { useState } from 'react'; import Image from 'next/image'; import { useFetch } from '@gitroom/helpers/utils/custom.fetch'; import interClass from '@gitroom/react/helpers/inter.font'; import { useVariables } from '@gitroom/react/helpers/variable.context'; export const OauthProvider = () => { const fetch = useFetch(); const { oauthLogoUrl, oauthDisplayName } = useVariables(); + const [error, setError] = useState<string | null>(null); const gotoLogin = useCallback(async () => { try { + setError(null); const response = await fetch('/auth/oauth/GENERIC'); if (!response.ok) { throw new Error( `Login link request failed with status ${response.status}` ); } const link = await response.text(); window.location.href = link; } catch (error) { console.error('Failed to get generic oauth login link:', error); + setError('Failed to initiate login. Please try again later.'); } }, []);
🧹 Nitpick comments (10)
apps/frontend/src/components/auth/login.tsx (1)
68-70: Added conditional rendering for the generic OAuth provider.The conditional rendering logic correctly shows the
OauthProvidercomponent when bothisGeneralandgenericOauthare true. Consider adding a fallback UI element or message when the provider is not available to improve user experience.{isGeneral && genericOauth ? ( <OauthProvider /> - ) : !isGeneral ? ( + ) : !isGeneral ? ( <GithubProvider /> ) : ( <div className="gap-[5px] flex flex-col"> <GoogleProvider /> {!!neynarClientId && <FarcasterProvider />} {billingEnabled && <WalletProvider />} </div> )}apps/frontend/src/app/layout.tsx (1)
42-44: Consider providing fallback values for OAuth environment variablesThe current implementation uses non-null assertion operators (
!) foroauthLogoUrlandoauthDisplayName. If these environment variables are undefined in a self-hosted environment, this could lead to unexpected behavior.- oauthLogoUrl={process.env.NEXT_PUBLIC_POSTIZ_OAUTH_LOGO_URL!} - oauthDisplayName={process.env.NEXT_PUBLIC_POSTIZ_OAUTH_DISPLAY_NAME!} + oauthLogoUrl={process.env.NEXT_PUBLIC_POSTIZ_OAUTH_LOGO_URL || ''} + oauthDisplayName={process.env.NEXT_PUBLIC_POSTIZ_OAUTH_DISPLAY_NAME || ''}apps/frontend/src/components/auth/providers/oauth.provider.tsx (2)
26-40: Improve accessibility of the login buttonThe current implementation uses a div with onClick which isn't fully accessible. Consider using a proper button element with appropriate ARIA attributes.
- return ( - <div - onClick={gotoLogin} - className={`cursor-pointer bg-white h-[44px] rounded-[4px] flex justify-center items-center text-customColor16 ${interClass} gap-[4px]`} - > + return ( + <button + onClick={gotoLogin} + aria-label={`Sign in with ${oauthDisplayName || 'OAuth'}`} + className={`cursor-pointer bg-white h-[44px] rounded-[4px] flex justify-center items-center text-customColor16 ${interClass} gap-[4px] border-0`} + > <div> <Image src={oauthLogoUrl || '/icons/generic-oauth.svg'} alt="genericOauth" width={40} height={40} /> </div> <div>Sign in with {oauthDisplayName || 'OAuth'}</div> + {error && <div className="text-red-500 text-sm mt-2">{error}</div>} - </div> + </button> );
11-14: Add loading state to improve user experienceThe current implementation doesn't indicate to users that a request is in progress, which could lead to confusion if the request takes time.
import { useCallback } from 'react'; -import { useState } from 'react'; +import { useState } from 'react'; import Image from 'next/image'; import { useFetch } from '@gitroom/helpers/utils/custom.fetch'; import interClass from '@gitroom/react/helpers/inter.font'; import { useVariables } from '@gitroom/react/helpers/variable.context'; export const OauthProvider = () => { const fetch = useFetch(); const { oauthLogoUrl, oauthDisplayName } = useVariables(); const [error, setError] = useState<string | null>(null); + const [isLoading, setIsLoading] = useState(false); const gotoLogin = useCallback(async () => { try { + setIsLoading(true); setError(null);Then update the button render logic to display a loading indicator:
<div> - <div>Sign in with {oauthDisplayName || 'OAuth'}</div> + <div>{isLoading ? 'Loading...' : `Sign in with ${oauthDisplayName || 'OAuth'}`}</div> {error && <div className="text-red-500 text-sm mt-2">{error}</div>}Don't forget to update the catch block:
} catch (error) { console.error('Failed to get generic oauth login link:', error); setError('Failed to initiate login. Please try again later.'); + setIsLoading(false); }.env.example (2)
95-99: Fix domain inconsistency in OAuth URLsThere's an inconsistency in the OAuth endpoint domains. The auth and token URLs use
auth.example.comwhile the userinfo URL usesauthentik.example.com.POSTIZ_GENERIC_OAUTH="false" POSTIZ_OAUTH_URL="https://auth.example.com" POSTIZ_OAUTH_AUTH_URL="https://auth.example.com/application/o/authorize" POSTIZ_OAUTH_TOKEN_URL="https://auth.example.com/application/o/token" -POSTIZ_OAUTH_USERINFO_URL="https://authentik.example.com/application/o/userinfo" +POSTIZ_OAUTH_USERINFO_URL="https://auth.example.com/application/o/userinfo"
93-102: Improve documentation for OAuth configurationThe current environment variables lack clear documentation about which ones are required vs. optional and how they relate to each other.
Consider adding more descriptive comments to guide users through the OAuth setup:
+# === OAuth Configuration +# Set POSTIZ_GENERIC_OAUTH to "true" to enable generic OAuth authentication NEXT_PUBLIC_POSTIZ_OAUTH_DISPLAY_NAME="Authentik" NEXT_PUBLIC_POSTIZ_OAUTH_LOGO_URL="https://raw.githubusercontent.com/walkxcode/dashboard-icons/master/png/authentik.png" POSTIZ_GENERIC_OAUTH="false" +# Base URL for your OAuth provider POSTIZ_OAUTH_URL="https://auth.example.com" +# Required: OAuth authorization endpoint POSTIZ_OAUTH_AUTH_URL="https://auth.example.com/application/o/authorize" +# Required: OAuth token endpoint POSTIZ_OAUTH_TOKEN_URL="https://auth.example.com/application/o/token" +# Required: OAuth userinfo endpoint POSTIZ_OAUTH_USERINFO_URL="https://authentik.example.com/application/o/userinfo" +# Required: Your OAuth client credentials POSTIZ_OAUTH_CLIENT_ID="" POSTIZ_OAUTH_CLIENT_SECRET="" +# Optional: Override default OAuth scopes # POSTIZ_OAUTH_SCOPE="openid profile email" # default valueslibraries/react-shared-libraries/src/helpers/variable.context.tsx (2)
8-10: Document newly added OAuth propertiesConsider adding JSDoc comments to explain the purpose of the new OAuth properties in the interface.
interface VariableContextInterface { billingEnabled: boolean; isGeneral: boolean; + /** Flag indicating if generic OAuth authentication is enabled */ genericOauth: boolean; + /** URL to the OAuth provider's logo image */ oauthLogoUrl: string; + /** Display name of the OAuth provider shown in the UI */ oauthDisplayName: string; frontEndUrl: string;
3-67: Consider grouping related properties for better organizationThe interface now has multiple OAuth-related properties mixed with other context properties. Consider grouping them for better organization and maintenance.
While this isn't an urgent change, here's how you could group related properties in the future:
interface VariableContextInterface { // UI and branding settings isGeneral: boolean; tolt: string; facebookPixel: string; // Authentication settings genericOauth: boolean; oauthLogoUrl: string; oauthDisplayName: string; // URL and endpoint settings frontEndUrl: string; backendUrl: string; discordUrl: string; uploadDirectory: string; // Feature flags billingEnabled: boolean; isSecured: boolean; // Integration keys plontoKey: string; neynarClientId: string; telegramBotName: string; // Configuration storageProvider: 'local' | 'cloudflare'; }apps/backend/src/services/auth/providers/oauth.provider.ts (2)
62-85: Robustness of token exchange
- JSON parsing is unchecked – if the IdP returns non-JSON error responses
await response.json()will throw and mask the earlier diagnostic.- The response may contain additional attributes (
token_type,expires_in,refresh_token). Throwing them away makes future refresh flows harder.- Hard-coding the same
redirect_uristring in two methods risks divergence.Refactor idea:
- const { access_token } = await response.json(); + let payload: any; + try { + payload = await response.json(); + } catch { + throw new Error('Token endpoint returned non-JSON payload'); + } + const { access_token } = payload; + if (!access_token) { + throw new Error('Token endpoint did not return access_token'); + }Also extract the redirect URI to a private getter or constant so both methods share it.
87-102: Graceful handling when email is missing & trailing-slash gotcha• Not all OIDC providers guarantee
preferred_usernameor return it only when the• You append
'/'afterthis.userInfoUrl, which will double-slash if the env value already ends with/. Considernew URL('userinfo', base)or trim trailing slashes.- const response = await fetch(`${this.userInfoUrl}/`, { + const response = await fetch(`${this.userInfoUrl.replace(/\/$/, '')}`, {Would you like a quick script to scan the codebase for similar double-slash concatenations?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/public/icons/generic-oauth.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
.env.example(3 hunks)apps/backend/src/services/auth/providers/oauth.provider.ts(1 hunks)apps/backend/src/services/auth/providers/providers.factory.ts(2 hunks)apps/frontend/src/app/layout.tsx(1 hunks)apps/frontend/src/components/auth/login.tsx(3 hunks)apps/frontend/src/components/auth/providers/oauth.provider.tsx(1 hunks)apps/frontend/src/middleware.ts(1 hunks)libraries/nestjs-libraries/src/database/prisma/schema.prisma(2 hunks)libraries/react-shared-libraries/src/helpers/variable.context.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/backend/src/services/auth/providers/providers.factory.ts (1)
apps/backend/src/services/auth/providers/oauth.provider.ts (1)
OauthProvider(3-103)
🔇 Additional comments (5)
libraries/nestjs-libraries/src/database/prisma/schema.prisma (2)
633-640: Added new Provider enum value for generic OAuth integration.The addition of the
GENERICenum value to theProviderenum is a necessary schema change to support the new generic OAuth/OIDC integration. This change correctly extends the authentication options available in the application.
648-653: Improved code formatting with added newline.Adding the newline after the closing brace of the
APPROVED_SUBMIT_FOR_ORDERenum improves code readability and follows best practices for file formatting.apps/backend/src/services/auth/providers/providers.factory.ts (1)
7-7: Imported OauthProvider class for generic OIDC support.The import of the
OauthProviderclass is necessary to support the generic OAuth/OIDC functionality requested in the PR.apps/frontend/src/components/auth/login.tsx (2)
12-12: Imported OauthProvider component for the generic SSO integration.This import enables the application to use the new generic OAuth provider component in the login flow.
28-29: Added genericOauth flag to component variables.Including the
genericOauthflag from the context is necessary to conditionally render the appropriate authentication provider.
| case Provider.GENERIC: | ||
| return new OauthProvider(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Added support for the GENERIC provider type.
The case for Provider.GENERIC correctly instantiates the OauthProvider class when requested, following the factory pattern consistently with other providers. Based on the relevant code snippet, make sure all required environment variables (POSTIZ_OAUTH_*) are documented in the deployment guide.
🏁 Script executed:
#!/bin/bash
# Verify if the environment variables are documented in the example file
echo "Checking for OAuth environment variables in .env.example"
fd -t f ".env.example" | xargs cat | grep -E "POSTIZ_OAUTH_"Length of output: 181
🏁 Script executed:
#!/bin/bash
# Search for POSTIZ_OAUTH_ references across the entire repository
echo "🔍 Searching for POSTIZ_OAUTH_ across all files"
rg -n "POSTIZ_OAUTH_" .
# List all .env* files to ensure we catch variants like .env.sample, .env.development, etc.
echo "📂 Listing all .env* files"
fd -t f "^\.env" || echo "No .env* files found"
# Check each .env* file for POSTIZ_OAUTH_ entries
echo "🔎 Inspecting .env* files for POSTIZ_OAUTH_"
fd -t f "^\.env" | while read -r file; do
echo "➡️ File: $file"
grep -H "POSTIZ_OAUTH_" "$file" || echo " └─ No matches in $file"
done
# Search documentation directories (e.g., docs/, .github/) for environment variable mentions
echo "📖 Searching in docs/ and .github/"
rg -n "POSTIZ_OAUTH_" docs .github || echo "No documentation entries found for POSTIZ_OAUTH_"Length of output: 3939
Please document the new POSTIZ_OAUTH_ variables in your environment examples and deployment guide*
The OauthProvider now relies on six POSTIZ_OAUTH_* variables, but I couldn’t find any .env-style example or docs entries listing them. Before merging, please:
• Add the following keys to your .env.example (or equivalent) with placeholder values:
- POSTIZ_OAUTH_AUTH_URL
- POSTIZ_OAUTH_URL
- POSTIZ_OAUTH_TOKEN_URL
- POSTIZ_OAUTH_CLIENT_ID
- POSTIZ_OAUTH_CLIENT_SECRET
- POSTIZ_OAUTH_USERINFO_URL
• Update your deployment or onboarding documentation (e.g., in your docs/ directory or README) to explain each variable’s purpose and expected format.
That will ensure users know how to configure the new GENERIC provider correctly.
|
Still nothing ? Been a bit frustrating.. to make some work upon a features ask by he community and still waiting some news from the maintener after several month (first PR included ) .. |
|
@nevo-david please review this! |
|
Alright, because Nevo doesnt seem to respond to this, im just going to merge it as it seems like a solid addition and it looks good to me. |
thank you , will provide asap (today or tomorrow what do you means by @egelhaus PR doc ready to be review =) => gitroomhq/postiz-docs#98 |
|
Hey, with Redirect URI I meant the URI to put in the OIDC Provider, and that you provide it here so we can use it as soon as you say it. |
|
Possible to disable user registration and only use sso/oidc? |
|
No, currently not, disabling User registration disables OIDC |
|
okay thanks |
Add Generic sso/OIDC
6 participants
What kind of change does this PR introduce?
Features
Reopen becasue #550 was accidentally closed
Why was this change needed?
add new oidc features for self hosted stuff
Please link to related issues when possible, and explain WHY you changed things, not WHAT you changed.
Other information:
Related to #344
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
TODO:
drummyfloyd/postiz:gh-550for authentik provider onlydrummyfloyd/postiz:gh-550-genericSummary by CodeRabbit