fix(oidc): remove trailling / needed but add by User & Missing Oauth on /auth #736
Conversation
depends on provider keycloack != authentik
|
@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 update OAuth provider handling in both backend and frontend components. The backend removes trailing slashes from OAuth-related URLs in service calls, while the frontend introduces conditional rendering of a generic OAuth provider during registration, based on new variable flags, replacing the previous unconditional Google provider rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RegisterAfter (Frontend)
participant OauthProvider (Frontend)
participant GoogleProvider (Frontend)
User->>RegisterAfter (Frontend): Access registration
RegisterAfter (Frontend)->>RegisterAfter (Frontend): Check variables (genericOauth, isGeneral)
alt genericOauth and isGeneral are true
RegisterAfter (Frontend)->>OauthProvider (Frontend): Render generic OAuth option
else
RegisterAfter (Frontend)->>GoogleProvider (Frontend): Render Google OAuth option
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/frontend/src/components/auth/register.tsxOops! Something went wrong! :( ESLint: 8.57.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@eslint/eslintrc' imported from /eslint.config.mjs apps/backend/src/services/auth/providers/oauth.provider.tsOops! Something went wrong! :( ESLint: 8.57.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@eslint/eslintrc' imported from /eslint.config.mjs Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
DrummyFloyd
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/backend/src/services/auth/providers/oauth.provider.ts (1)
51-60: Consider adding clear documentation about URL formatting requirementsThe URL formatting changes are subtle but impactful. Consider adding a comment in the constructor or by the environment variable validation to explain that users need to manually include the trailing slash if their OIDC provider requires it.
this.tokenUrl = POSTIZ_OAUTH_TOKEN_URL; this.userInfoUrl = POSTIZ_OAUTH_USERINFO_URL; + // Note: Some OIDC providers (e.g., Keycloak) require trailing slashes in URLs, + // while others (e.g., Authentik) do not. Please add trailing slashes manually + // to the environment variables if your provider requires them. }apps/frontend/src/components/auth/register.tsx (1)
168-175: Consider adding aria-label to improve accessibilityFor better accessibility, consider adding descriptive aria-labels to the provider buttons to ensure screen readers can properly identify the authentication options.
<div className="gap-[5px] flex flex-col"> {genericOauth && isGeneral ? ( - <OauthProvider /> + <OauthProvider aria-label="Continue with Single Sign-On" /> ) : ( - <GoogleProvider /> + <GoogleProvider aria-label="Continue with Google" /> )} {!!neynarClientId && <FarcasterProvider />} {billingEnabled && <WalletProvider />} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backend/src/services/auth/providers/oauth.provider.ts(2 hunks)apps/frontend/src/components/auth/register.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/backend/src/services/auth/providers/oauth.provider.ts (1)
libraries/nestjs-libraries/src/integrations/social.abstract.ts (1)
fetch(23-65)
apps/frontend/src/components/auth/register.tsx (2)
libraries/react-shared-libraries/src/helpers/variable.context.tsx (1)
useVariables(59-61)apps/frontend/src/components/auth/providers/google.provider.tsx (1)
GoogleProvider(5-45)
🔇 Additional comments (5)
apps/backend/src/services/auth/providers/oauth.provider.ts (2)
63-63: Trailing slash removed to allow provider-specific URL formattingThis change aligns with the PR objective of allowing manual trailing slash control for different OIDC providers like Keycloak and Authentik that handle URL formats differently.
88-88: Trailing slash removed from userInfoUrl for consistent provider handlingSimilar to the tokenUrl change, this ensures the OIDC userInfo endpoint URL formatting is determined by the environment variable value set by the user, improving flexibility across different providers.
apps/frontend/src/components/auth/register.tsx (3)
17-17: Added import for generic OAuth providerThis import enables the new OIDC login option mentioned in the PR objectives.
95-96: Added genericOauth variable to configurationCorrectly updated the variables destructuring to include the new
genericOauthflag required for conditional OAuth provider rendering.
168-172: Implemented conditional rendering of OAuth providersThis change successfully adds the OIDC button on the
/authroute as required by the PR objectives, showing either the generic OAuth provider or Google provider based on configuration.
| {genericOauth && isGeneral ? ( | ||
| <OauthProvider /> | ||
| ) : ( | ||
| <GoogleProvider /> | ||
| )} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify OauthProvider component implementation
Make sure the OauthProvider component follows the same pattern as GoogleProvider with proper error handling and UI consistency.
🏁 Script executed:
#!/bin/bash
# Find and display the OauthProvider component implementation
fd "oauth.provider.tsx" --exec cat {}Length of output: 1329
🏁 Script executed:
#!/bin/bash
# Find and display the GoogleProvider component implementation
rg -A20 "export const GoogleProvider" --glob "*.tsx"Length of output: 2016
Align error handling and UI between OauthProvider and GoogleProvider
The OauthProvider implementation correctly adds a try/catch and checks response.ok, but the GoogleProvider still uses a “bare” fetch call and renders a smaller icon. To keep both providers consistent:
• apps/frontend/src/components/auth/providers/google.provider.tsx
- Wrap the fetch in try/catch and check
response.okbefore reading text, mirroringOauthProvider. - Example diff:
export const GoogleProvider = () => { const fetch = useFetch(); -
const gotoLogin = useCallback(async () => { -
const link = await (await fetch('/auth/oauth/GOOGLE')).text(); -
window.location.href = link; -
}, []);
-
const gotoLogin = useCallback(async () => { -
try { -
const response = await fetch('/auth/oauth/GOOGLE'); -
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 Google login link:', error); -
} -
}, []);
• Icon sizing & accessibility
OauthProvideruses a 40×40 Image;GoogleProvider’s SVG is 21×21. Pick one size (e.g. 40×40) and apply it to both for visual parity.- Ensure both have meaningful
alttext for screen readers.
Once these changes are in place, both components will share identical fetch patterns, error handling and UI styling.
Committable suggestion skipped: line range outside the PR's diff.
fix(oidc): remove trailling / needed but add by User & Missing Oauth on /auth
depends on provider keycloack != authentik# What kind of change does this PR introduce?
Issue on
/OIDC env configneed to be set manually by user, each provider set this differently (keycloack != Authentik)
add OIDC button on
/authWhy was this change needed?
discuss with @egelhaus
Other information:
can be tested with image from docker.hub =>
drummyfloyd/postiz:fix-oidcChecklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit
New Features
Bug Fixes