Skip to content
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.

Sign up for GitHub

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

fix / sso: make sure to delete only loginToken after redirect #16415

Merged
merged 4 commits into from
Feb 11, 2021
Merged

fix / sso: make sure to delete only loginToken after redirect #16415

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e57378
fix / sso: after complete login via token make sure to delete only lo…
bekliev Feb 9, 2021
92a5787
Migrating from Node's deprecated `url` API to browser's `URL`
bekliev Feb 11, 2021
e15d1c1
Update src/vector/platform/WebPlatform.ts
t3chguy Feb 11, 2021
438eef0
Update src/vector/app.tsx
t3chguy Feb 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@
"react": "^16.14.0",
"react-dom": "^16.14.0",
"sanitize-html": "github:apostrophecms/sanitize-html#3c7f93f2058f696f5359e3e58d464161647226db",
"ua-parser-js": "^0.7.23",
"url": "^0.11.0"
"ua-parser-js": "^0.7.23"
},
"devDependencies": {
"@babel/core": "^7.12.10",
Expand Down
12 changes: 6 additions & 6 deletions src/vector/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import React from 'react';
// this incidentally means we can forget our React imports in JSX files without penalty.
window.React = React;

import url from 'url';
import * as sdk from 'matrix-react-sdk';
import PlatformPeg from 'matrix-react-sdk/src/PlatformPeg';
import {_td, newTranslatableError} from 'matrix-react-sdk/src/languageHandler';
Expand Down Expand Up @@ -120,11 +119,12 @@ function onTokenLoginCompleted() {
// if we did a token login, we're now left with the token, hs and is
// url as query params in the url; a little nasty but let's redirect to
// clear them.
const parsedUrl = url.parse(window.location.href);
parsedUrl.search = "";
const formatted = url.format(parsedUrl);
console.log(`Redirecting to ${formatted} to drop loginToken from queryparams`);
window.history.replaceState(null, "", formatted);
const url = new URL(window.location.href);

url.searchParams.delete('loginToken');
t3chguy marked this conversation as resolved.
Show resolved Hide resolved

console.log(`Redirecting to ${url.href} to drop loginToken from queryparams`);
window.history.replaceState(null, "", url.href);
}

export async function loadApp(fragParams: {}) {
Expand Down
19 changes: 7 additions & 12 deletions src/vector/platform/WebPlatform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {hideToast as hideUpdateToast, showToast as showUpdateToast} from "matrix
import {Action} from "matrix-react-sdk/src/dispatcher/actions";
import { CheckUpdatesPayload } from 'matrix-react-sdk/src/dispatcher/payloads/CheckUpdatesPayload';

import url from 'url';
import UAParser from 'ua-parser-js';

const POKE_RATE_MS = 10 * 60 * 1000; // 10 min
Expand Down Expand Up @@ -184,17 +183,13 @@ export default class WebPlatform extends VectorBasePlatform {

getDefaultDeviceDisplayName(): string {
// strip query-string and fragment from uri
const u = url.parse(window.location.href);
u.protocol = "";
u.search = "";
u.hash = "";
// Remove trailing slash if present
u.pathname = u.pathname.replace(/\/$/, "");
Comment on lines -191 to -192
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it does look like the behaviour here has been lost.

Previously it'd pass through things like riot.im/app happily too but I think now that'll instead just be riot.im

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... will check that out soon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it can be fixed with this (commiting this approach now)
image


let appName = u.format();
// Remove leading slashes if present
appName = appName.replace(/^\/\//, "");
// `appName` is now in the format `develop.element.io`.
const url = new URL(window.location.href);

// `appName` in the format `develop.element.io/abc/xyz`
const appName = [
url.host,
url.pathname.replace(/\/$/, ""), // Remove trailing slash if present
].join('');
t3chguy marked this conversation as resolved.
Show resolved Hide resolved

const ua = new UAParser();
const browserName = ua.getBrowser().name || "unknown browser";
Expand Down
11 changes: 1 addition & 10 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8890,16 +8890,7 @@ postcss-attribute-case-insensitive@^4.0.1:
postcss "^7.0.2"
postcss-selector-parser "^6.0.2"

postcss-calc@^7.0.1:
version "7.0.5"
resolved "https://registry.yarnpkg.com/postcss-calc/-/postcss-calc-7.0.5.tgz#f8a6e99f12e619c2ebc23cf6c486fdc15860933e"
integrity sha512-1tKHutbGtLtEZF6PT4JSihCHfIVldU72mZ8SdZHIYriIZ9fh9k9aWSppaT8rHsyI3dX+KSR+W+Ix9BMY3AODrg==
dependencies:
postcss "^7.0.27"
postcss-selector-parser "^6.0.2"
postcss-value-parser "^4.0.2"

postcss-calc@^7.0.5:
postcss-calc@^7.0.1, postcss-calc@^7.0.5:
version "7.0.5"
resolved "https://registry.yarnpkg.com/postcss-calc/-/postcss-calc-7.0.5.tgz#f8a6e99f12e619c2ebc23cf6c486fdc15860933e"
integrity sha512-1tKHutbGtLtEZF6PT4JSihCHfIVldU72mZ8SdZHIYriIZ9fh9k9aWSppaT8rHsyI3dX+KSR+W+Ix9BMY3AODrg==
Expand Down