-
Notifications
You must be signed in to change notification settings - Fork 392
Updated login page appearance #9425
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 1 commit
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 |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import React from "react"; | ||
import { PluginLoginMethodSelection, LoginConfig } from "../pluginLoginMethodSelection"; | ||
|
||
class DefaultLoginMethodSelectionPlugin implements PluginLoginMethodSelection { | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
renderMethodSelection(_loginConfig: LoginConfig | undefined): React.ReactElement | null { | ||
return null; | ||
} | ||
} | ||
|
||
export default new DefaultLoginMethodSelectionPlugin(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import React from "react"; | ||
|
||
export interface LoginConfig { | ||
login_url: string; | ||
username_ui_placeholder: string; | ||
password_ui_placeholder: string; | ||
login_failed_message?: string; | ||
fallback_login_url?: string; | ||
fallback_login_label?: string; | ||
login_cookie_names: string[]; | ||
logout_url: string; | ||
select_login_method?: boolean; | ||
} | ||
|
||
export interface PluginLoginMethodSelection { | ||
renderMethodSelection: (loginConfig: LoginConfig | undefined) => React.ReactElement | null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,39 +64,31 @@ const TopNavLink = ({ href, children }) => { | |
}; | ||
|
||
const TopNav = ({logged = true}) => { | ||
if (!logged) { | ||
if (logged) { | ||
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. What happens if 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. @itaigilo In the new login page, when But as requested by Barak, I’ll split this PR to two PR, this one will handle the design changes and the other will cover the plugin functionality. 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. |
||
return ( | ||
<Navbar variant="dark" bg="dark" expand="md"> | ||
<Container fluid={true}> | ||
<Link component={Navbar.Brand} href="/"> | ||
<img src="/logo.png" alt="lakeFS" className="logo"/> | ||
</Link> | ||
</Container> | ||
<Navbar variant="dark" bg="dark" expand="md" className="border-bottom"> | ||
<Container fluid={true}> | ||
<Link component={Navbar.Brand} href="/"> | ||
<img src="/logo.png" alt="lakeFS" className="logo"/> | ||
</Link> | ||
<Navbar.Toggle aria-controls="navbarScroll" /> | ||
<Navbar.Collapse id="navbarScroll"> | ||
|
||
<Nav className="me-auto my-2 my-lg-0" | ||
style={{ maxHeight: '100px' }} | ||
navbarScroll> | ||
<TopNavLink href="/repositories">Repositories</TopNavLink> | ||
<TopNavLink href="/auth">Administration</TopNavLink> | ||
</Nav> | ||
|
||
<DarkModeToggle/> | ||
<NavUserInfo/> | ||
</Navbar.Collapse> | ||
</Container> | ||
</Navbar> | ||
); | ||
} | ||
return ( | ||
<Navbar variant="dark" bg="dark" expand="md" className="border-bottom"> | ||
<Container fluid={true}> | ||
<Link component={Navbar.Brand} href="/"> | ||
<img src="/logo.png" alt="lakeFS" className="logo"/> | ||
</Link> | ||
<Navbar.Toggle aria-controls="navbarScroll" /> | ||
<Navbar.Collapse id="navbarScroll"> | ||
|
||
<Nav className="me-auto my-2 my-lg-0" | ||
style={{ maxHeight: '100px' }} | ||
navbarScroll> | ||
<TopNavLink href="/repositories">Repositories</TopNavLink> | ||
<TopNavLink href="/auth">Administration</TopNavLink> | ||
</Nav> | ||
|
||
<DarkModeToggle/> | ||
<NavUserInfo/> | ||
</Navbar.Collapse> | ||
</Container> | ||
</Navbar> | ||
); | ||
return null; | ||
}; | ||
|
||
export default TopNav; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,19 @@ | ||
import React, {useState} from "react"; | ||
import Row from "react-bootstrap/Row"; | ||
import Card from "react-bootstrap/Card"; | ||
import Form from "react-bootstrap/Form"; | ||
import Col from "react-bootstrap/Col"; | ||
import Button from "react-bootstrap/Button"; | ||
import {auth, AuthenticationError, setup, SETUP_STATE_INITIALIZED} from "../../lib/api"; | ||
import {AlertError} from "../../lib/components/controls" | ||
import {useRouter} from "../../lib/hooks/router"; | ||
import {useAPI} from "../../lib/hooks/api"; | ||
import {useNavigate} from "react-router-dom"; | ||
import {usePluginManager} from "../../extendable/plugins/pluginsContext"; | ||
|
||
interface SetupResponse { | ||
state: string; | ||
comm_prefs_missing?: boolean; | ||
login_config?: LoginConfig; | ||
} | ||
|
||
interface LoginConfig { | ||
login_url: string; | ||
|
@@ -19,111 +24,142 @@ interface LoginConfig { | |
fallback_login_label?: string; | ||
login_cookie_names: string[]; | ||
logout_url: string; | ||
select_login_method?: boolean; | ||
} | ||
|
||
const LoginForm = ({loginConfig}: {loginConfig: LoginConfig}) => { | ||
const router = useRouter(); | ||
const navigate = useNavigate(); | ||
const [loginError, setLoginError] = useState(null); | ||
const [loginError, setLoginError] = useState<React.ReactNode>(null); | ||
const { next } = router.query; | ||
const usernamePlaceholder = loginConfig.username_ui_placeholder || "Access Key ID"; | ||
const passwordPlaceholder = loginConfig.password_ui_placeholder || "Secret Access Key"; | ||
|
||
return ( | ||
<Row> | ||
<Col md={{offset: 4, span: 4}}> | ||
<Card className="login-widget shadow-lg border-0"> | ||
<Card.Header className="text"> | ||
<h4 className="mb-0">Login</h4> | ||
</Card.Header> | ||
<Card.Body className="p-4"> | ||
<Form onSubmit={async (e) => { | ||
e.preventDefault() | ||
try { | ||
setLoginError(null); | ||
await auth.login(e.target.username.value, e.target.password.value) | ||
router.push(next || '/'); | ||
navigate(0); | ||
} catch(err) { | ||
if (err instanceof AuthenticationError && err.status === 401) { | ||
const contents = {__html: `${loginConfig.login_failed_message}` || | ||
"Credentials don't match."}; | ||
setLoginError(<span dangerouslySetInnerHTML={contents}/>); | ||
} | ||
<div className="d-flex align-items-center justify-content-center login-container"> | ||
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. The Just define for the It will place it at a fixed margin from the top, and center it horizontally. And in addition, vertical centering is tricky with html + css, and prone for errors. |
||
<Card className="login-widget shadow-lg border-0 login-card"> | ||
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. Why both |
||
<Card.Header className="text-center"> | ||
<div className="mb-3"> | ||
<img src="/logo.png" alt="lakeFS" className="login-logo" /> | ||
</div> | ||
<h4 className="mb-0">Login</h4> | ||
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. This is more for @nopcoder , I guess, but - |
||
</Card.Header> | ||
<Card.Body className="p-4"> | ||
<Form onSubmit={async (e) => { | ||
e.preventDefault() | ||
const form = e.target as HTMLFormElement; | ||
const formData = new FormData(form); | ||
try { | ||
setLoginError(null); | ||
const username = formData.get('username'); | ||
const password = formData.get('password'); | ||
if (typeof username === 'string' && typeof password === 'string') { | ||
await auth.login(username, password); | ||
} | ||
}}> | ||
<Form.Group controlId="username" className="mb-3"> | ||
<Form.Control | ||
type="text" | ||
placeholder={usernamePlaceholder} | ||
autoFocus | ||
className="bg-light" | ||
/> | ||
</Form.Group> | ||
|
||
<Form.Group controlId="password" className="mb-3"> | ||
<Form.Control | ||
type="password" | ||
placeholder={passwordPlaceholder} | ||
className="bg-light" | ||
/> | ||
</Form.Group> | ||
|
||
{(!!loginError) && <AlertError error={loginError}/>} | ||
|
||
<Button | ||
variant="primary" | ||
type="submit" | ||
className="w-100 mt-3 py-2" | ||
> | ||
Login | ||
</Button> | ||
</Form> | ||
<div className={"mt-2 mb-1"}> | ||
{ loginConfig.fallback_login_url ? | ||
<Button variant="link" className="text-secondary mt-2" onClick={async ()=> { | ||
loginConfig.login_cookie_names?.forEach( | ||
cookie => { | ||
document.cookie = `${cookie}=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;`; | ||
} | ||
); | ||
window.location = loginConfig.fallback_login_url; | ||
}}>{loginConfig.fallback_login_label || 'Try another way to login'}</Button> | ||
: "" | ||
router.push(next || '/'); | ||
navigate(0); | ||
} catch(err) { | ||
if (err instanceof AuthenticationError && err.status === 401) { | ||
const contents = {__html: `${loginConfig.login_failed_message}` || | ||
"Credentials don't match."}; | ||
setLoginError(<span dangerouslySetInnerHTML={contents}/>); | ||
} | ||
</div> | ||
</Card.Body> | ||
</Card> | ||
</Col> | ||
</Row> | ||
} | ||
}}> | ||
<Form.Group controlId="username" className="mb-3"> | ||
<Form.Control | ||
name="username" | ||
type="text" | ||
placeholder={usernamePlaceholder} | ||
autoFocus | ||
className="bg-light" | ||
/> | ||
</Form.Group> | ||
|
||
<Form.Group controlId="password" className="mb-3"> | ||
<Form.Control | ||
name="password" | ||
type="password" | ||
placeholder={passwordPlaceholder} | ||
className="bg-light" | ||
/> | ||
</Form.Group> | ||
|
||
{(!!loginError) && <AlertError error={loginError}/>} | ||
|
||
<Button | ||
variant="primary" | ||
type="submit" | ||
className="w-100 mt-3 py-2" | ||
> | ||
Login | ||
</Button> | ||
</Form> | ||
<div className={"mt-2 mb-1"}> | ||
{ loginConfig.fallback_login_url ? | ||
<Button variant="link" className="text-secondary mt-2" onClick={async ()=> { | ||
loginConfig.login_cookie_names?.forEach( | ||
cookie => { | ||
document.cookie = `${cookie}=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;`; | ||
} | ||
); | ||
if (loginConfig.fallback_login_url) { | ||
window.location.href = loginConfig.fallback_login_url; | ||
} | ||
}}>{loginConfig.fallback_login_label || 'Try another way to login'}</Button> | ||
: "" | ||
} | ||
</div> | ||
</Card.Body> | ||
</Card> | ||
</div> | ||
) | ||
} | ||
|
||
|
||
|
||
const LoginPage = () => { | ||
const router = useRouter(); | ||
const { response, error, loading } = useAPI(() => setup.getState()); | ||
const pluginManager = usePluginManager(); | ||
|
||
if (loading) { | ||
return null; | ||
} | ||
|
||
// if we are not initialized, or we are not done with comm prefs, redirect to 'setup' page | ||
if (!error && response && (response.state !== SETUP_STATE_INITIALIZED || response.comm_prefs_missing === true)) { | ||
router.push({pathname: '/setup', query: router.query}) | ||
if (!error && response && ((response as SetupResponse).state !== SETUP_STATE_INITIALIZED || (response as SetupResponse).comm_prefs_missing)) { | ||
router.push({pathname: '/setup', params: {}, query: router.query as Record<string, string>}) | ||
return null; | ||
} | ||
const loginConfig = response?.login_config; | ||
const setupResponse = response as SetupResponse | null; | ||
const loginConfig = setupResponse?.login_config; | ||
|
||
const loginMethodSelectionComponent = loginConfig ? pluginManager.loginMethodSelection.renderMethodSelection(loginConfig) : null; | ||
if (loginMethodSelectionComponent) { | ||
if (router.query.method === 'local' && loginConfig) { | ||
return <LoginForm loginConfig={loginConfig}/>; | ||
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. If you can ensure LoginConfig isn't false-y, remove |
||
} | ||
|
||
return loginMethodSelectionComponent; | ||
} | ||
|
||
if (router.query.redirected) { | ||
if(!error && loginConfig?.login_url) { | ||
window.location = loginConfig.login_url; | ||
window.location.href = loginConfig.login_url; | ||
return null; | ||
} | ||
delete router.query.redirected; | ||
|
||
router.push({pathname: '/auth/login', query: router.query}) | ||
router.push({pathname: '/auth/login', params: {}, query: router.query as Record<string, string>}) | ||
} | ||
if (!loginConfig) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<LoginForm loginConfig={loginConfig}/> | ||
); | ||
}; | ||
|
||
export default LoginPage; | ||
export default LoginPage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,4 +91,25 @@ body .auth-page .nav-pills .nav-link:hover { | |
/* Direct style override for nav-pills */ | ||
.auth-page .nav-pills .nav-link.active { | ||
background-color: var(--success) !important; | ||
} | ||
} | ||
|
||
.login-container { | ||
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. What is |
||
height: 100vh; | ||
overflow: hidden; | ||
position: fixed; | ||
top: 0; | ||
left: 0; | ||
width: 100%; | ||
background-color: #f8f9fa; | ||
} | ||
|
||
.login-card { | ||
max-width: 600px; | ||
width: 90%; | ||
} | ||
|
||
.login-logo { | ||
width: 170px; | ||
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. These are wrong values: |
||
height: 60px; | ||
filter: brightness(0) saturate(100%); | ||
} |
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.
Following my proposal in the other PR,
And plugins will check the method string to know which component to return
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.
@yonipeleg33
thanks I renamed it to:
renderLoginMethodComponent
and the Plugin name to:
PluginLoginMethod
and the default plugin imp to:
DefaultLoginMethodPlugin
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.
@yonipeleg33
This PR previously included changes that I moved to this PR. It now contains only appearance-related updates and TypeScript error fixes. The comments provided here have been addressed in the other PR.