Skip to content

Commit

Permalink
[server/login] handle errors on login
Browse files Browse the repository at this point in the history 
* already taken email addresses should be rejected on signup
* OAuth errors (e.g. access_denied) should be propagated
  • Loading branch information
@AlexTugarev
AlexTugarev committed Apr 15, 2021
1 parent d0ac11a commit 81d7ff4
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 111 deletions.
2 changes: 1 addition & 1 deletion ...s/dashboard/public/flow-result/index.html → ...dashboard/public/complete-auth/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@
</script>
</head>
<body>
This browser tab is supposed to close automatically.
If this tab is not closed automatically, feel free to close it and proceed. <button type="button" onclick="window.open('', '_self', ''); window.close();">Close</button>
</body>
</html>
105 changes: 59 additions & 46 deletions components/dashboard/src/Login.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
import { useContext, useEffect, useState } from "react";
import { UserContext } from "./user-context";
import { getGitpodService, gitpodHostUrl } from "./service/service";
import { iconForAuthProvider, simplifyProviderName } from "./provider-utils";
import { getGitpodService } from "./service/service";
import { iconForAuthProvider, openAuthorizeWindow, simplifyProviderName } from "./provider-utils";
import gitpod from './images/gitpod.svg';
import gitpodIcon from './icons/gitpod.svg';
import automate from "./images/welcome/automate.svg";
Expand All @@ -17,10 +17,10 @@ import collaborate from "./images/welcome/collaborate.svg";
import customize from "./images/welcome/customize.svg";
import fresh from "./images/welcome/fresh.svg";
import prebuild from "./images/welcome/prebuild.svg";
import exclamation from "./images/exclamation.svg";



function Item(props: {icon: string, iconSize?: string, text:string}) {
function Item(props: { icon: string, iconSize?: string, text: string }) {
const iconSize = props.iconSize || 28;
return <div className="flex-col items-center w-1/3 px-3">
<img src={props.icon} className={`w-${iconSize} m-auto h-24`} />
Expand All @@ -29,7 +29,7 @@ function Item(props: {icon: string, iconSize?: string, text:string}) {
}

export function markLoggedIn() {
document.cookie = "gitpod-user=loggedIn;max-age=" + 60*60*24*365;
document.cookie = "gitpod-user=loggedIn;max-age=" + 60 * 60 * 24 * 365;
}

export function hasLoggedInBefore() {
Expand All @@ -41,40 +41,49 @@ export function Login() {
const showWelcome = !hasLoggedInBefore();

const [authProviders, setAuthProviders] = useState<AuthProviderInfo[]>([]);
const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined);

useEffect(() => {
(async () => {
setAuthProviders(await getGitpodService().server.getAuthProviders());
})();
}, [])

const listener = (event: MessageEvent<any>) => {
// todo: check event.origin

if (event.data === "success") {
if (event.source && "close" in event.source && event.source.close) {
console.log(`try to close window`);
event.source.close();
const openLogin = async (host: string) => {
setErrorMessage(undefined);

try {
await openAuthorizeWindow({
login: true,
host,
onSuccess: () => updateUser(),
onError: (error) => {
if (typeof error === "string") {
try {
const payload = JSON.parse(error);
if (typeof payload === "object" && payload.error) {
if (payload.error === "email_taken") {
return setErrorMessage(`Email address already exists. Log in using a different provider.`);
}
return setErrorMessage(payload.description ? payload.description : `Error: ${payload.error}`);
}
} catch (error) {
console.log(error);
}
setErrorMessage(error);
}
}

(async () => {
await getGitpodService().reconnect();
setUser(await getGitpodService().server.getLoggedInUser());
markLoggedIn();
})();
}
};
window.addEventListener("message", listener);
return () => {
window.removeEventListener("message", listener);
});
} catch (error) {
console.log(error)
}
}, [])
}

const openLogin = (host: string) => {
const url = getLoginUrl(host);
const newWindow = window.open(url, "gitpod-login");
if (!newWindow) {
console.log(`Failed to open login window for ${host}`);
}
const updateUser = async () => {
await getGitpodService().reconnect();
const user = await getGitpodService().server.getLoggedInUser();
setUser(user);
markLoggedIn();
}

return (<div id="login-container" className="z-50 flex w-screen h-screen">
Expand All @@ -91,18 +100,18 @@ export function Login() {
</div>
</div>
<div className="flex mb-10">
<Item icon={code} iconSize="16" text="Always Ready&#x2011;To&#x2011;Code"/>
<Item icon={customize} text="Personalize your Workspace"/>
<Item icon={automate} text="Automate Your Development Setup"/>
<Item icon={code} iconSize="16" text="Always Ready&#x2011;To&#x2011;Code" />
<Item icon={customize} text="Personalize your Workspace" />
<Item icon={automate} text="Automate Your Development Setup" />
</div>
<div className="flex">
<Item icon={prebuild} text="Continuously Prebuild Your Project"/>
<Item icon={collaborate} text="Collaborate With Your Team"/>
<Item icon={fresh} text="Fresh Workspace For Each New Task"/>
<Item icon={prebuild} text="Continuously Prebuild Your Project" />
<Item icon={collaborate} text="Collaborate With Your Team" />
<Item icon={fresh} text="Fresh Workspace For Each New Task" />
</div>
</div>
</div>
</div>: null}
</div> : null}
<div id="login-section" className={"flex-grow flex w-full" + (showWelcome ? " lg:w-1/2" : "")}>
<div id="login-section-column" className={"flex-grow max-w-2xl flex flex-col h-100 mx-auto" + (showWelcome ? " lg:my-0" : "")}>
<div className="flex-grow h-100 flex flex-row items-center justify-center" >
Expand All @@ -111,7 +120,7 @@ export function Login() {
<img src={gitpodIcon} className="h-16 mx-auto" />
</div>
<div className="mx-auto text-center pb-8 space-y-2">
<h1 className="text-3xl">Log in{showWelcome? ' to Gitpod' : ''}</h1>
<h1 className="text-3xl">Log in{showWelcome ? ' to Gitpod' : ''}</h1>
<h2 className="uppercase text-sm text-gray-400">ALWAYS READY-TO-CODE</h2>
</div>
<div className="flex flex-col space-y-3 items-center">
Expand All @@ -124,6 +133,18 @@ export function Login() {
);
})}
</div>

{errorMessage && (
<div className="mt-16 flex space-x-2 py-6 px-6 w-96 justify-between bg-gitpod-kumquat-light rounded-xl">
<div className="pr-3 self-center w-6">
<img src={exclamation} />
</div>
<div className="flex-1 flex flex-col">
<p className="text-gitpod-red text-sm">{errorMessage}</p>
</div>
</div>
)}

</div>
</div>
<div className="flex-none mx-auto h-20 text-center">
Expand All @@ -136,11 +157,3 @@ export function Login() {
</div>
</div>);
}

function getLoginUrl(host: string) {
const returnTo = gitpodHostUrl.with({ pathname: 'flow-result', search: 'message=success' }).toString();
return gitpodHostUrl.withApi({
pathname: '/login',
search: `host=${host}&returnTo=${encodeURIComponent(returnTo)}`
}).toString();
}
2 changes: 1 addition & 1 deletion components/dashboard/src/prebuilds/InstallGitHubApp.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function registerApp(installationId: string, setModal: (modal: 'done' | st
try {
await getGitpodService().server.registerGithubApp(installationId);

const returnTo = encodeURIComponent(gitpodHostUrl.with({ pathname: 'flow-result', search: 'message=success' }).toString());
const returnTo = encodeURIComponent(gitpodHostUrl.with({ pathname: 'complete-auth', search: 'message=success' }).toString());
const url = gitpodHostUrl.withApi({
pathname: '/authorize',
search: `returnTo=${returnTo}&host=github.com&scopes=repo`
Expand Down
32 changes: 23 additions & 9 deletions components/dashboard/src/provider-utils.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,26 @@ function simplifyProviderName(host: string) {
}
}

async function openAuthorizeWindow({ host, scopes, onSuccess, onError }: { host: string, scopes?: string[], onSuccess?: (payload?: string) => void, onError?: (error?: string) => void }) {
const returnTo = gitpodHostUrl.with({ pathname: 'flow-result', search: 'message=success' }).toString();
const url = gitpodHostUrl.withApi({
pathname: '/authorize',
search: `returnTo=${encodeURIComponent(returnTo)}&host=${host}&override=true&scopes=${(scopes || []).join(',')}`
}).toString();
interface OpenAuthorizeWindowParams {
login?: boolean;
host: string;
scopes?: string[];
onSuccess?: (payload?: string) => void;
onError?: (error?: string) => void;
}

async function openAuthorizeWindow(params: OpenAuthorizeWindowParams) {
const { login, host, scopes, onSuccess, onError } = params;
const returnTo = gitpodHostUrl.with({ pathname: 'complete-auth', search: 'message=success' }).toString();
const url = login
? gitpodHostUrl.withApi({
pathname: '/login',
search: `host=${host}&returnTo=${encodeURIComponent(returnTo)}`
}).toString()
: gitpodHostUrl.withApi({
pathname: '/authorize',
search: `returnTo=${encodeURIComponent(returnTo)}&host=${host}&override=true&scopes=${(scopes || []).join(',')}`
}).toString();

const newWindow = window.open(url, "gitpod-auth-window");
if (!newWindow) {
Expand All @@ -55,7 +69,7 @@ async function openAuthorizeWindow({ host, scopes, onSuccess, onError }: { host:

const killAuthWindow = () => {
window.removeEventListener("message", eventListener);

if (event.source && "close" in event.source && event.source.close) {
console.log(`Received Auth Window Result. Closing Window.`);
event.source.close();
Expand All @@ -67,9 +81,9 @@ async function openAuthorizeWindow({ host, scopes, onSuccess, onError }: { host:
onSuccess && onSuccess();
}
if (typeof event.data === "string" && event.data.startsWith("error:")) {
const errorText = atob(event.data.substring("error:".length));
const errorAsText = atob(event.data.substring("error:".length));
killAuthWindow();
onError && onError(errorText);
onError && onError(errorAsText);
}
};
window.addEventListener("message", eventListener);
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/settings/Integrations.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function GitProviders() {

const disconnect = async (ap: AuthProviderInfo) => {
setDisconnectModal(undefined);
const returnTo = gitpodHostUrl.with({ pathname: 'flow-result', search: 'message=success' }).toString();
const returnTo = gitpodHostUrl.with({ pathname: 'complete-auth', search: 'message=success' }).toString();
const deauthorizeUrl = gitpodHostUrl.withApi({
pathname: '/deauthorize',
search: `returnTo=${returnTo}&host=${ap.host}`
Expand Down
13 changes: 13 additions & 0 deletions components/server/src/auth/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,16 @@ export namespace SelectAccountException {
return AuthException.is(error) && error.authException === type;
}
}

export interface EmailAddressAlreadyTakenException extends AuthException {
payload: string;
}
export namespace EmailAddressAlreadyTakenException {
const type = "EmailAddressAlreadyTakenException";
export function create(message: string) {
return AuthException.create(type, message, message);
}
export function is(error: any): error is EmailAddressAlreadyTakenException {
return AuthException.is(error) && error.authException === type;
}
}
48 changes: 33 additions & 15 deletions components/server/src/auth/generic-auth-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ import { AuthProviderInfo, Identity, Token, User } from '@gitpod/gitpod-protocol
import { log, LogContext } from '@gitpod/gitpod-protocol/lib/util/logging';
import fetch from "node-fetch";
import { oauth2tokenCallback, OAuth2 } from 'oauth';
import { format as formatURL, URL } from 'url';
import { URL } from 'url';
import { runInNewContext } from "vm";
import { AuthFlow, AuthProvider } from "../auth/auth-provider";
import { AuthProviderParams, AuthUserSetup } from "../auth/auth-provider";
import { AuthException, SelectAccountException } from "../auth/errors";
import { AuthException, EmailAddressAlreadyTakenException, SelectAccountException } from "../auth/errors";
import { GitpodCookie } from "./gitpod-cookie";
import { SelectAccountCookie } from "../user/select-account-cookie";
import { Env } from "../env";
import { getRequestingClientInfo } from "../express-util";
import { TokenProvider } from '../user/token-provider';
Expand Down Expand Up @@ -64,7 +63,6 @@ export class GenericAuthProvider implements AuthProvider {
@inject(UserDB) protected userDb: UserDB;
@inject(Env) protected env: Env;
@inject(GitpodCookie) protected gitpodCookie: GitpodCookie;
@inject(SelectAccountCookie) protected selectAccountCookie: SelectAccountCookie;
@inject(UserService) protected readonly userService: UserService;
@inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
@inject(LoginCompletionHandler) protected readonly loginCompletionHandler: LoginCompletionHandler;
Expand Down Expand Up @@ -297,17 +295,17 @@ export class GenericAuthProvider implements AuthProvider {
const defaultLogPayload = { authFlow, clientInfo, authProviderId, request };

// check OAuth2 errors
const error = new URL(formatURL({ protocol: request.protocol, host: request.get('host'), pathname: request.originalUrl })).searchParams.get("error");
const callbackParams = new URL(`https://anyhost${request.originalUrl}`).searchParams;
const error = callbackParams.get("error");
const description: string | null = callbackParams.get("error_description");

if (error) { // e.g. "access_denied"
// Clean up the session
await AuthFlow.clear(request.session);
await TosFlow.clear(request.session);

increaseLoginCounter("failed", this.host);

log.info(cxt, `(${strategyName}) Received OAuth2 error, thus redirecting to /sorry (${error})`, { ...defaultLogPayload, requestUrl: request.originalUrl });
response.redirect(this.getSorryUrl(`OAuth2 error. (${error})`));
return;
return this.sendCompletionRedirectWithError(response, { error, description });
}

let result: Parameters<VerifyCallback>;
Expand Down Expand Up @@ -347,12 +345,10 @@ export class GenericAuthProvider implements AuthProvider {
await TosFlow.clear(request.session);

if (SelectAccountException.is(err)) {
this.selectAccountCookie.set(response, err.payload);

// option 1: send as GET param on redirect
const url = this.env.hostUrl.with({ pathname: '/flow-result', search: "message=error:" + Buffer.from(JSON.stringify(err.payload), "utf-8").toString('base64') }).toString();
response.redirect(url);
return;
return this.sendCompletionRedirectWithError(response, err.payload);
}
if (EmailAddressAlreadyTakenException.is(err)) {
return this.sendCompletionRedirectWithError(response, { error: "email_taken" });
}

let message = 'Authorization failed. Please try again.';
Expand Down Expand Up @@ -394,6 +390,13 @@ export class GenericAuthProvider implements AuthProvider {
}
}

protected sendCompletionRedirectWithError(response: express.Response, error: object): void {
log.info(`(${this.strategyName}) Send completion redirect with error`, { error });

const url = this.env.hostUrl.with({ pathname: '/complete-auth', search: "message=error:" + Buffer.from(JSON.stringify(error), "utf-8").toString('base64') }).toString();
response.redirect(url);
}

/**
* cf. part (2) of `callback` function
*
Expand Down Expand Up @@ -435,6 +438,21 @@ export class GenericAuthProvider implements AuthProvider {
} else {
// no user session present, let's initiate a login
currentGitpodUser = await this.userService.findUserForLogin({ candidate });

if (!currentGitpodUser) {

// signup new accounts with email adresses already taken is disallowed
const existingUserWithSameEmail = (await this.userDb.findUsersByEmail(primaryEmail))[0];
if (existingUserWithSameEmail) {
try {
await this.userService.asserNoAccountWithEmail(primaryEmail);
} catch (error) {
log.warn(`Login attempt with matching email address.`, { ...defaultLogPayload, authUser, candidate, clientInfo });
done(error, undefined);
return;
}
}
}
}

const token = this.createToken(this.tokenUsername, accessToken, refreshToken, currentScopes, tokenResponse.expires_in);
Expand Down
2 changes: 0 additions & 2 deletions components/server/src/container-module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import { MonitoringEndpointsApp } from './monitoring-endpoints';
import { BearerAuth } from './auth/bearer-authenticator';
import { TermsProvider } from './terms/terms-provider';
import { TosCookie } from './user/tos-cookie';
import { SelectAccountCookie } from './user/select-account-cookie';
import { ContentServiceClient } from '@gitpod/content-service/lib/content_grpc_pb';
import { BlobServiceClient } from '@gitpod/content-service/lib/blobs_grpc_pb';
import { WorkspaceServiceClient } from '@gitpod/content-service/lib/workspace_grpc_pb';
Expand All @@ -90,7 +89,6 @@ export const productionContainerModule = new ContainerModule((bind, unbind, isBo
bind(LoginCompletionHandler).toSelf().inSingletonScope();
bind(GitpodCookie).toSelf().inSingletonScope();
bind(TosCookie).toSelf().inSingletonScope();
bind(SelectAccountCookie).toSelf().inSingletonScope();

bind(SessionHandlerProvider).toSelf().inSingletonScope();
bind(Server).toSelf().inSingletonScope();
Expand Down
Loading

0 comments on commit 81d7ff4

Please sign in to comment.