Skip to content

Commit

Permalink
Fix cookie handling in Firefox when using HTTP (#5160)
Browse files Browse the repository at this point in the history
* Fix cookie handling in Firefox when using HTTP

* Fix test
  • Loading branch information
Sebastian Florek committed May 21, 2020
1 parent 50abf41 commit 53115e0
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 44 deletions.
39 changes: 38 additions & 1 deletion aio/gulp/conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,40 @@ const version = {
*/
const imageNameBase = 'dashboard';

function envToArgv() {
let envArgs = process.env.DASHBOARD_ARGS;
let result = {};

const args = minimist(process.argv.splice(2));
delete args._;
Object.keys(args).forEach(key => result[key] = args[key]);

if(!envArgs) {
return result;
}

envArgs = envArgs.split(';');
if(envArgs.length === 0) {
return result;
}

envArgs.forEach(arg => {
const parts = arg.split('=');
if(parts.length === 2) {
result[parts[0]] = parts[1];
return;
}

result[parts[0]] = true;
})

return result;
}

/**
* Arguments
*/
const argv = minimist(process.argv.slice(2));
const argv = envToArgv();

/**
* Exported configuration object with common constants used in build pipeline.
Expand Down Expand Up @@ -184,6 +214,13 @@ export default {
enableSkipButton: argv.enableSkipButton !== undefined ?
argv.enableSkipButton :
false,

/**
* Allows to enable login view when serving on http.
*/
enableInsecureLogin: argv.enableInsecureLogin !== undefined ?
argv.enableInsecureLogin :
false,
},

/**
Expand Down
6 changes: 2 additions & 4 deletions aio/gulp/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ function getBackendArgs() {
`--tls-cert-file=${conf.backend.tlsCert}`,
`--tls-key-file=${conf.backend.tlsKey}`,
`--auto-generate-certificates=${conf.backend.autoGenerateCerts}`,
`--enable-insecure-login=${conf.backend.enableInsecureLogin}`,
`--enable-skip-login=${conf.backend.enableSkipButton}`
];

if (conf.backend.systemBanner.length > 0) {
Expand All @@ -58,10 +60,6 @@ function getBackendArgs() {
args.push(`--apiserver-host=${conf.backend.apiServerHost}`);
}

if (conf.backend.enableSkipButton) {
args.push(`--enable-skip-login=${conf.backend.enableSkipButton}`);
}

return args;
}

Expand Down
66 changes: 36 additions & 30 deletions src/app/frontend/common/services/global/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@ import {KdStateService} from './state';
export class AuthService {
private readonly _config = CONFIG;

get allowedProtocol(): string {
return 'https';
}

get domainWhitelist(): string[] {
return ['localhost', '127.0.0.1'];
}

constructor(
private readonly cookies_: CookieService,
private readonly router_: Router,
Expand All @@ -61,34 +53,40 @@ export class AuthService {
}

private setTokenCookie_(token: string): void {
// This will only work for HTTPS connection
this.cookies_.set(this._config.authTokenCookieName, token, null, null, null, true, 'Strict');
// This will only work when accessing Dashboard at 'localhost' or
// '127.0.0.1'
this.cookies_.set(
this._config.authTokenCookieName,
token,
null,
null,
'localhost',
false,
'Strict',
);
this.cookies_.set(
this._config.authTokenCookieName,
token,
null,
null,
'127.0.0.1',
false,
'Strict',
);
if (!this.isLoginEnabled()) {
return;
}

if (this.isCurrentProtocolSecure_()) {
this.cookies_.set(this._config.authTokenCookieName, token, null, null, null, true, 'Strict');
return;
}

if (this.isCurrentDomainSecure_()) {
this.cookies_.set(
this._config.authTokenCookieName,
token,
null,
null,
location.hostname,
false,
'Strict',
);
}
}

private getTokenCookie_(): string {
return this.cookies_.get(this._config.authTokenCookieName) || '';
}

private isCurrentDomainSecure_(): boolean {
return ['localhost', '127.0.0.1'].indexOf(location.hostname) > -1;
}

private isCurrentProtocolSecure_(): boolean {
return location.protocol.includes('https');
}

removeAuthCookies(): void {
this.cookies_.delete(this._config.authTokenCookieName);
this.cookies_.delete(this._config.skipLoginPageCookieName);
Expand Down Expand Up @@ -193,4 +191,12 @@ export class AuthService {
isLoginPageEnabled(): boolean {
return !(this.cookies_.get(this._config.skipLoginPageCookieName) === 'true');
}

/**
* Returns true if domain is localhost/127.0.0.1 or if the connection
* protocol is HTTPS, false otherwise.
*/
isLoginEnabled(): boolean {
return this.isCurrentDomainSecure_() || this.isCurrentProtocolSecure_();
}
}
8 changes: 2 additions & 6 deletions src/app/frontend/login/component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,8 @@ class MockAuthService {

skipLoginPage(): void {}

get allowedProtocol(): string {
return 'https';
}

get domainWhitelist(): string[] {
return ['localhost', '127.0.0.1'];
isLoginEnabled(): boolean {
return true;
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/app/frontend/login/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,7 @@ export class LoginComponent implements OnInit {
}

isLoginEnabled(): boolean {
return this.authService_.domainWhitelist.indexOf(location.hostname) > -1
? true
: location.protocol === this.authService_.allowedProtocol;
return this.authService_.isLoginEnabled();
}

onChange(event: Event & KdFile): void {
Expand Down

0 comments on commit 53115e0

Please sign in to comment.