Skip to content

Commit

Permalink
Fix frontend not redirecting on 401 (#1244)
Browse files Browse the repository at this point in the history
- Ensures need-login event bubbles until handled
- Redirects on 401 from /refresh endpoint
- Go to previous URL upon login, rather than always to home page
- Shows accurate login notification (rather than less precise "couldn't retrieve org" or similar message)
  • Loading branch information
SuaYoo authored Oct 4, 2023
1 parent 38efecc commit f2261bc
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 15 deletions.
33 changes: 22 additions & 11 deletions frontend/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { NotifyEvent, NavigateEvent } from "./utils/LiteElement";
import LiteElement, { html } from "./utils/LiteElement";
import APIRouter from "./utils/APIRouter";
import AuthService, { AuthState } from "./utils/AuthService";
import type { LoggedInEvent } from "./utils/AuthService";
import type { LoggedInEvent, NeedLoginEvent } from "./utils/AuthService";
import type { ViewState } from "./utils/APIRouter";
import type { CurrentUser, UserOrg } from "./types/user";
import type { AuthStorageEventData } from "./utils/AuthService";
Expand Down Expand Up @@ -107,6 +107,7 @@ export class App extends LiteElement {
}
super.connectedCallback();

window.addEventListener("need-login", this.onNeedLogin);
window.addEventListener("popstate", () => {
this.syncViewState();
});
Expand Down Expand Up @@ -585,7 +586,8 @@ export class App extends LiteElement {
@logged-in=${this.onLoggedIn}
.authState=${this.authService.authState}
.viewState=${this.viewState}
redirectUrl=${this.viewState.params.redirectUrl}
redirectUrl=${this.viewState.params.redirectUrl ||
this.viewState.data?.redirectUrl}
></btrix-log-in>`;

case "resetPassword":
Expand Down Expand Up @@ -616,7 +618,6 @@ export class App extends LiteElement {
return html`<btrix-orgs
class="w-full md:bg-neutral-50"
@navigate="${this.onNavigateTo}"
@need-login="${this.onNeedLogin}"
.authState="${this.authService.authState}"
.userInfo="${this.userInfo}"
></btrix-orgs>`;
Expand All @@ -632,7 +633,6 @@ export class App extends LiteElement {
return html`<btrix-org
class="w-full"
@navigate=${this.onNavigateTo}
@need-login=${this.onNeedLogin}
@update-user-info=${(e: CustomEvent) => {
e.stopPropagation();
this.updateUserInfo();
Expand All @@ -653,7 +653,6 @@ export class App extends LiteElement {
class="w-full max-w-screen-lg mx-auto p-2 md:py-8 box-border"
@navigate="${this.onNavigateTo}"
@logged-in=${this.onLoggedIn}
@need-login="${this.onNeedLogin}"
.authState="${this.authService.authState}"
.userInfo="${this.userInfo}"
></btrix-account-settings>`;
Expand All @@ -665,7 +664,6 @@ export class App extends LiteElement {
class="w-full max-w-screen-lg mx-auto p-2 md:py-8 box-border"
@navigate="${this.onNavigateTo}"
@logged-in=${this.onLoggedIn}
@need-login="${this.onNeedLogin}"
.authState="${this.authService.authState}"
.userInfo="${this.userInfo}"
></btrix-users-invite>`;
Expand All @@ -684,7 +682,6 @@ export class App extends LiteElement {
return html`<btrix-crawls
class="w-full"
@navigate=${this.onNavigateTo}
@need-login=${this.onNeedLogin}
@notify=${this.onNotify}
.authState=${this.authService.authState}
crawlId=${this.viewState.params.crawlId}
Expand Down Expand Up @@ -781,7 +778,7 @@ export class App extends LiteElement {
this.clearUser();

if (redirect) {
this.navigate("/log-in");
this.navigate(ROUTES.login);
}
}

Expand All @@ -805,10 +802,24 @@ export class App extends LiteElement {
this.updateUserInfo();
}

onNeedLogin() {
onNeedLogin = (e: Event) => {
e.stopPropagation();

this.clearUser();
this.navigate(ROUTES.login);
}
const redirectUrl = (e as NeedLoginEvent).detail?.redirectUrl;
this.navigate(ROUTES.login, {
redirectUrl,
});
this.onNotify(
new CustomEvent("notify", {
detail: {
message: msg("Please log in to continue."),
variant: "warning" as any,
icon: "exclamation-triangle",
},
})
);
};

onNavigateTo(event: NavigateEvent) {
event.stopPropagation();
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/org/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export class Org extends LiteElement {
}

async willUpdate(changedProperties: Map<string, any>) {
if (changedProperties.has("orgId") && this.orgId) {
if (changedProperties.has("orgId") && this.orgId && this.authState) {
try {
this.org = await this.getOrg(this.orgId);
this.checkStorageQuota();
Expand Down
26 changes: 26 additions & 0 deletions frontend/src/utils/AuthService.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ROUTES } from "../routes";
import { APIError } from "./api";

export type Auth = {
Expand Down Expand Up @@ -27,6 +28,14 @@ export interface LoggedInEvent<T = LoggedInEventDetail> extends CustomEvent {
readonly detail: T;
}

export interface NeedLoginEvent extends CustomEvent {
readonly bubbles: boolean;
readonly composed: boolean;
readonly detail: {
redirectUrl?: string;
};
}

type AuthRequestEventData = {
name: "requesting_auth";
};
Expand All @@ -51,6 +60,7 @@ export default class AuthService {
static storageKey = "btrix.auth";
static unsupportedAuthErrorCode = "UNSUPPORTED_AUTH_TYPE";
static loggedInEvent = "logged-in";
static needLoginEvent = "need-login";

static broadcastChannel = new BroadcastChannel(AuthService.storageKey);
static storage = {
Expand Down Expand Up @@ -85,6 +95,14 @@ export default class AuthService {
return new CustomEvent(AuthService.loggedInEvent, { detail });
};

static createNeedLoginEvent = (redirectUrl?: string): NeedLoginEvent => {
return new CustomEvent(AuthService.needLoginEvent, {
bubbles: true,
composed: true,
detail: { redirectUrl },
});
};

static async login({
email,
password,
Expand Down Expand Up @@ -307,6 +325,14 @@ export default class AuthService {
}, FRESHNESS_TIMER_INTERVAL);
} catch (e) {
console.debug(e);

this.logout();
const { pathname, search, hash } = window.location;
const redirectUrl =
pathname !== ROUTES.login && pathname !== ROUTES.home
? `${pathname}${search}${hash}`
: "";
window.dispatchEvent(AuthService.createNeedLoginEvent(redirectUrl));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/utils/LiteElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { TemplateResult } from "lit";
import { msg } from "@lit/localize";

import type { Auth } from "../utils/AuthService";
import AuthService from "../utils/AuthService";
import { APIError } from "./api";

export interface NavigateEvent extends CustomEvent {
Expand Down Expand Up @@ -147,7 +148,7 @@ export default class LiteElement extends LitElement {

switch (resp.status) {
case 401: {
this.dispatchEvent(new CustomEvent("need-login"));
this.dispatchEvent(AuthService.createNeedLoginEvent());
errorMessage = msg("Need login");
break;
}
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/utils/auth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import LiteElement from "../utils/LiteElement";
import type { AuthState } from "../utils/AuthService";
import type { CurrentUser } from "../types/user";
import AuthService from "../utils/AuthService";

/**
* Block rendering and dispatch event if user is not logged in
Expand All @@ -27,7 +27,11 @@ export function needLogin<T extends { new (...args: any[]): LiteElement }>(
if (this.authState) {
super.update(changedProperties);
} else {
this.dispatchEvent(new CustomEvent("need-login"));
this.dispatchEvent(
AuthService.createNeedLoginEvent(
`${window.location.pathname}${window.location.search}${window.location.hash}`
)
);
}
}
};
Expand Down

0 comments on commit f2261bc

Please sign in to comment.