-
-
Notifications
You must be signed in to change notification settings - Fork 636
Add OIDC/OAuth SSO support (rebased from #719) #916
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
base: master
Are you sure you want to change the base?
Changes from all commits
162aaba
0494091
5b31ba6
491d218
d7f82f1
d0fc617
f8827b2
5d701b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { useExtracted } from "next-intl"; | |
| import { useState } from "react"; | ||
| import { authClient } from "../../../lib/auth"; | ||
| import { userStore } from "../../../lib/userStore"; | ||
| import { useConfigs } from "../../../lib/configs"; | ||
| import { AuthInput } from "@/components/auth/AuthInput"; | ||
| import { AuthButton } from "@/components/auth/AuthButton"; | ||
| import { AuthError } from "@/components/auth/AuthError"; | ||
|
|
@@ -19,6 +20,21 @@ export function Login({ callbackURL }: LoginProps) { | |
| const [error, setError] = useState<string>(""); | ||
| const [email, setEmail] = useState(""); | ||
| const [password, setPassword] = useState(""); | ||
| const { configs } = useConfigs(); | ||
|
|
||
| const handleSSOLogin = async () => { | ||
| if (!configs?.enabledOIDCProviders.length) return; | ||
|
|
||
| const provider = configs.enabledOIDCProviders[0]; | ||
| try { | ||
| await authClient.signIn.oauth2({ | ||
| providerId: provider.providerId, | ||
| callbackURL, | ||
| }); | ||
| } catch (err) { | ||
| setError(String(err)); | ||
| } | ||
| }; | ||
|
Comment on lines
+25
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's examine the login.tsx file to understand the full context
cat -n client/src/app/invitation/components/login.tsx | head -120Repository: rybbit-io/rybbit Length of output: 3757 🏁 Script executed: # Search for how SocialButtons renders multiple buttons per provider
rg -n "SocialButtons\|providerId\|enabledOIDCProviders" --type=ts --type=tsx -B 2 -A 5 client/Repository: rybbit-io/rybbit Length of output: 87 🏁 Script executed: # Check the PR or commit message to understand the claimed feature
git log --oneline --all -n 50 2>/dev/null | head -20
git show --stat 2>/dev/null | head -100Repository: rybbit-io/rybbit Length of output: 7829 🏁 Script executed: # Find and examine the SocialButtons component
find client -name "*SocialButtons*" -o -name "*social*" | head -20Repository: rybbit-io/rybbit Length of output: 104 🏁 Script executed: # Search for SocialButtons implementation
rg -l "SocialButtons" client/ --type tsRepository: rybbit-io/rybbit Length of output: 260 🏁 Script executed: # Check the auth components directory
ls -la client/components/auth/ 2>/dev/null || echo "Not found, searching..."
fd -t f "SocialButtons" client/Repository: rybbit-io/rybbit Length of output: 128 🏁 Script executed: # Search for how providers are rendered or selected
rg "enabledOIDCProviders|SocialButtons" --type ts -B 3 -A 8 client/Repository: rybbit-io/rybbit Length of output: 13437 🏁 Script executed: # Check git commit message for PR 719 to see claimed features
git log --grep="OIDC\|oauth\|provider" --oneline -n 20 2>/dev/null || git log --all --oneline | grep -i "oidc\|oauth\|provider" | head -10Repository: rybbit-io/rybbit Length of output: 42 🏁 Script executed: # Get full SocialButtons implementation
cat -n client/src/components/auth/SocialButtons.tsxRepository: rybbit-io/rybbit Length of output: 3482 🏁 Script executed: # Check if invitation login.tsx already has SocialButtons rendering on line 71
sed -n '60,110p' client/src/app/invitation/components/login.tsxRepository: rybbit-io/rybbit Length of output: 1305 Remove the redundant The The separate 🤖 Prompt for AI Agents |
||
|
|
||
| const handleLogin = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
|
|
@@ -53,27 +69,41 @@ export function Login({ callbackURL }: LoginProps) { | |
| <form onSubmit={handleLogin}> | ||
| <div className="flex flex-col gap-4"> | ||
| <SocialButtons onError={setError} callbackURL={callbackURL} /> | ||
| <AuthInput | ||
| id="email" | ||
| label={t("Email")} | ||
| type="email" | ||
| placeholder="example@email.com" | ||
| required | ||
| value={email} | ||
| onChange={e => setEmail(e.target.value)} | ||
| /> | ||
| <AuthInput | ||
| id="password" | ||
| label={t("Password")} | ||
| type="password" | ||
| placeholder="••••••••" | ||
| required | ||
| value={password} | ||
| onChange={e => setPassword(e.target.value)} | ||
| /> | ||
| <AuthButton isLoading={isLoading} loadingText={t("Logging in...")}> | ||
| {t("Login to Accept Invitation")} | ||
| </AuthButton> | ||
| {configs?.internalAuthEnabled && ( | ||
| <> | ||
| <AuthInput | ||
| id="email" | ||
| label={t("Email")} | ||
| type="email" | ||
| placeholder="example@email.com" | ||
| required | ||
| value={email} | ||
| onChange={e => setEmail(e.target.value)} | ||
| /> | ||
| <AuthInput | ||
| id="password" | ||
| label={t("Password")} | ||
| type="password" | ||
| placeholder="••••••••" | ||
| required | ||
| value={password} | ||
| onChange={e => setPassword(e.target.value)} | ||
| /> | ||
| <AuthButton isLoading={isLoading} loadingText={t("Logging in...")}> | ||
| {t("Login to Accept Invitation")} | ||
| </AuthButton> | ||
| </> | ||
| )} | ||
| {configs?.enabledOIDCProviders.length ? ( | ||
| <AuthButton | ||
| isLoading={false} | ||
| type="button" | ||
| variant="default" | ||
| onClick={handleSSOLogin} | ||
| > | ||
| Login with SSO | ||
| </AuthButton> | ||
|
Comment on lines
+99
to
+105
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing SSO-specific loading state and hardcoded UI string violates i18n Two issues in the SSO button:
As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n". ♻️ Proposed fixAdd a dedicated loading state for SSO: const [isLoading, setIsLoading] = useState(false);
+ const [isSSOLoading, setIsSSOLoading] = useState(false);Update const handleSSOLogin = async () => {
if (!configs?.enabledOIDCProviders?.length) return;
const provider = configs.enabledOIDCProviders[0];
+ setIsSSOLoading(true);
try {
await authClient.signIn.oauth2({
providerId: provider.providerId,
callbackURL,
});
} catch (err) {
setError(String(err));
+ } finally {
+ setIsSSOLoading(false);
}
};Update the button: - <AuthButton
- isLoading={false}
- type="button"
- variant="default"
- onClick={handleSSOLogin}
- >
- Login with SSO
- </AuthButton>
+ <AuthButton
+ isLoading={isSSOLoading}
+ type="button"
+ variant="default"
+ onClick={handleSSOLogin}
+ >
+ {t("Login with SSO")}
+ </AuthButton>Remember to add the key to all translation files ( 🤖 Prompt for AI Agents |
||
| ) : null} | ||
| <AuthError error={error} /> | ||
| </div> | ||
| </form> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,7 @@ import { Turnstile } from "@/components/auth/Turnstile"; | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useExtracted } from "next-intl"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import Link from "next/link"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useRouter } from "next/navigation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useEffect, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { RybbitTextLogo } from "../../components/RybbitLogo"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { SpinningGlobe } from "../../components/SpinningGlobe"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useSetPageTitle } from "../../hooks/useSetPageTitle"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -73,6 +73,19 @@ export default function Page() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const turnstileEnabled = IS_CLOUD && process.env.NODE_ENV === "production"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Auto-redirect to SSO if internal auth is disabled and there's only one OIDC provider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!isLoadingConfigs && configs && !configs.internalAuthEnabled && configs.enabledOIDCProviders.length === 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const provider = configs.enabledOIDCProviders[0]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| authClient.signIn.oauth2({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| providerId: provider.providerId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callbackURL: "/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }).catch(err => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setError(String(err)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [configs, isLoadingConfigs]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
+87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe Two issues in this
🛡️ Proposed fix+ const [isAutoRedirecting, setIsAutoRedirecting] = useState(false);
useEffect(() => {
- if (!isLoadingConfigs && configs && !configs.internalAuthEnabled && configs.enabledOIDCProviders.length === 1) {
+ if (!isLoadingConfigs && configs && !configs.internalAuthEnabled && configs.enabledOIDCProviders?.length === 1) {
const provider = configs.enabledOIDCProviders[0];
+ setIsAutoRedirecting(true);
authClient.signIn.oauth2({
providerId: provider.providerId,
callbackURL: "/",
}).catch(err => {
setError(String(err));
+ setIsAutoRedirecting(false);
});
}
}, [configs, isLoadingConfigs]);Then render a loading indicator when 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex h-dvh w-full"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {/* Left panel - login form */} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -87,55 +100,57 @@ export default function Page() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| <h1 className="text-lg text-neutral-600 dark:text-neutral-300 mb-6">{t("Welcome back")}</h1> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex flex-col gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SocialButtons onError={setError} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <form onSubmit={handleSubmit}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex flex-col gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthInput | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id="email" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label={t("Email")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type="email" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| placeholder="example@email.com" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={email} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={e => setEmail(e.target.value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthInput | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id="password" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label={t("Password")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type="password" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| placeholder="••••••••" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={password} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={e => setPassword(e.target.value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rightElement={ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IS_CLOUD && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Link href="/reset-password" className="text-xs text-muted-foreground hover:text-primary"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("Forgot password?")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Link> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {turnstileEnabled && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Turnstile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSuccess={token => setTurnstileToken(token)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onError={() => setTurnstileToken("")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onExpire={() => setTurnstileToken("")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="flex justify-center" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {configs?.internalAuthEnabled && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <form onSubmit={handleSubmit}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex flex-col gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthInput | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id="email" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label={t("Email")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type="email" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| placeholder="example@email.com" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={email} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={e => setEmail(e.target.value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthButton | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isLoading={isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loadingText={t("Logging in...")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("Login")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </AuthButton> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthInput | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id="password" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label={t("Password")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type="password" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| placeholder="••••••••" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={password} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={e => setPassword(e.target.value)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rightElement={ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IS_CLOUD && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Link href="/reset-password" className="text-xs text-muted-foreground hover:text-primary"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("Forgot password?")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Link> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthError error={error} title={t("Error Logging In")} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </form> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {turnstileEnabled && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Turnstile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSuccess={token => setTurnstileToken(token)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onError={() => setTurnstileToken("")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onExpire={() => setTurnstileToken("")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="flex justify-center" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthButton | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isLoading={isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loadingText={t("Logging in...")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={turnstileEnabled ? !turnstileToken || isLoading : isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t("Login")} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </AuthButton> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <AuthError error={error} title={t("Error Logging In")} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </form> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {(!configs?.disableSignup || !isLoadingConfigs) && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="text-center text-sm"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsafe property access —
enabledOIDCProviders.lengthcan throw if the property is nullishconfigs?.enabledOIDCProviders.lengthshort-circuits toundefinedwhenconfigsitself is nullish, but ifconfigsis defined whileenabledOIDCProvidersisnullorundefined(e.g. during an in-flight config fetch or a partial response), the unguarded.lengthaccess throws aTypeError. The same pattern is repeated on line 93.🛡️ Proposed fix
🤖 Prompt for AI Agents