Skip to content

Commit

Permalink
Merge pull request #1515 from shadow-dot-cat/castaway/1097_redirect_p…
Browse files Browse the repository at this point in the history 
…ost_login_for_restricted_url

fix(login): Redirect to subscriptions after login if began there
  • Loading branch information
@castaway
castaway authored Dec 19, 2023
2 parents cd136e5 + f227822 commit 5317eb5
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 74 deletions.
37 changes: 22 additions & 15 deletions src/app/account-app/no-products-for-subaccounts.guard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, RouterStateSnapshot, UrlTree, CanActivateChild, CanActivate, Router } from '@angular/router';
import { AsyncSubject, Observable } from 'rxjs';
import { RunboxMe, RunboxWebmailAPI } from '../rmmapi/rbwebmail';
import { Observable } from 'rxjs';
import { RunboxWebmailAPI } from '../rmmapi/rbwebmail';
import { RMMAuthGuardService } from '../rmmapi/rmmauthguard.service';

import { AccountRenewalsComponent } from './account-renewals.component';
import { AccountTransactionsComponent } from './account-transactions.component';
Expand All @@ -38,30 +39,36 @@ export class NoProductsForSubaccountsGuard implements CanActivate, CanActivateCh
CreditCardsComponent,
];

me: AsyncSubject<RunboxMe>;

constructor(
rmmapi: RunboxWebmailAPI,
private rmmapi: RunboxWebmailAPI,
private authGuard: RMMAuthGuardService,
private router: Router,
) {
this.me = rmmapi.me;
}

canActivate(
route: ActivatedRouteSnapshot, _state: RouterStateSnapshot
): boolean | UrlTree | Observable<boolean | UrlTree> | Promise<boolean | UrlTree> {
const restricted = this.banned_components.find(c => route.component === c);
if (restricted) {
return this.me.toPromise().then(me => {
if (me.owner) {
return this.router.parseUrl('/account/not-for-subaccounts');
return this.authGuard.checkLogin().then(
(success) => {
if (typeof(success) === 'boolean' && success) {
const restricted = this.banned_components.find(c => route.component === c);
if (restricted) {
return this.rmmapi.me.toPromise().then(me => {
if (me.owner) {
return this.router.parseUrl('/account/not-for-subaccounts');
} else {
return true;
}
});
} else {
return true;
}
} else {
return true;
// Its a UrlTree or similar
return success;
}
});
} else {
return true;
}
}

canActivateChild(
Expand Down
7 changes: 2 additions & 5 deletions src/app/http/progress.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,8 @@ describe('ProgressService', () => {
take(1)
).subscribe(() => httpProgressSeen = true);

const req = httpMock.expectOne(`/rest/v1/me`);

req.flush({
'result': {'uid': '11', 'last_name': 'testuser'},
'status': 'success'}, {status: 200, statusText: 'OK'});
// We now expect login / httpauth to set the runboxme values
rmmapiservice.setRunboxMe({'uid': '11', 'last_name': 'testuser'});

const last_on_req = httpMock.expectOne(`/rest/v1/last_on`);
last_on_req.flush(200);
Expand Down
18 changes: 14 additions & 4 deletions src/app/login/login.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// along with Runbox 7. If not, see <https://www.gnu.org/licenses/>.
// ---------- END RUNBOX LICENSE ----------

import { Component, OnInit } from '@angular/core';
import { Component, OnInit, NgZone } from '@angular/core';
import { Router } from '@angular/router';

import { HttpClient } from '@angular/common/http';
Expand Down Expand Up @@ -46,12 +46,13 @@ export class LoginComponent implements OnInit {
unlock_question: string;
login_error_html: string;

constructor(private httpclient: HttpClient,
constructor(
private httpclient: HttpClient,
private router: Router,
private authservice: RMMAuthGuardService,
private ngZone: NgZone,
public progressService: ProgressService
) {

}

ngOnInit() {
Expand Down Expand Up @@ -164,7 +165,16 @@ export class LoginComponent implements OnInit {
window.location.href = '/mail';
return;
}
this.router.navigateByUrl(this.authservice.urlBeforeLogin);
this.ngZone.run(() => {
this.router.navigateByUrl(this.authservice.urlBeforeLogin,
{ replaceUrl: true })
.then((result) => {
if(!result) {
this.router.navigateByUrl('/', { replaceUrl: true });
}
})
.catch((e) => console.log('Navigate error: ' + e));
});
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/app/rmmapi/rbwebmail.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,10 @@ describe('RBWebMail', () => {
// this is a good enough cheat to have the actual request kick in early enough.
await new Promise(resolve => setTimeout(resolve, 0));

// We now expect login / httpauth to set the runboxme values
// so we set it directly here
rmmapi.setRunboxMe({'uid': '11', 'last_name': 'testuser'});
const httpTestingController = TestBed.inject(HttpTestingController);
httpTestingController.expectOne('/rest/v1/me').flush({
status: 'success',
result: { uid: 123 }
});
// HACK: crappy solution to get the email request to resolve
// see https://github.com/angular/angular/issues/25965
await new Promise(resolve => setTimeout(resolve, 500));
Expand Down
39 changes: 18 additions & 21 deletions src/app/rmmapi/rbwebmail.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,31 +181,28 @@ export class RunboxWebmailAPI {
public rmm: RMM,
) {
this.rblocale = new RunboxLocale();
this.http.get('/rest/v1/me')
.pipe(
map((res: any) => res.result),
map((res: any) => {
res.uid = parseInt(res.uid, 10);
res.disk_used = res.quotas ? parseInt(res.quotas.disk_used, 10) : null;
return new RunboxMe(res);
})
).subscribe((me: RunboxMe) => {
this.me.next(me);
this.me.complete();

this.ngZone.runOutsideAngular(() =>
this.last_on_interval = setInterval(() => this.ngZone.run(() => {
this.updateLastOn().subscribe();
}), 5 * 60 * 1000)
);

this.updateLastOn().subscribe();
});

this.me.subscribe(me => {
this.messageCache.next(new MessageCache(me.uid));
this.messageCache.complete();
});
});
}

public setRunboxMe(res:any) {
res.uid = parseInt(res.uid, 10);
res.disk_used = res.quotas ? parseInt(res.quotas.disk_used, 10) : null;
const me = new RunboxMe(res);
this.me.next(me);
this.me.complete();

if (!this.last_on_interval) {
this.ngZone.runOutsideAngular(() =>
this.last_on_interval = setInterval(() => this.ngZone.run(() => {
this.updateLastOn().subscribe();
}), 5 * 60 * 1000)
);
this.updateLastOn().subscribe();
}
}

public deleteCachedMessageContents(messageId: number) {
Expand Down
28 changes: 18 additions & 10 deletions src/app/rmmapi/rmmauthguard.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,32 @@ import { HttpClient } from '@angular/common/http';
import { CanActivate, ActivatedRouteSnapshot, RouterStateSnapshot, UrlTree, Router, CanActivateChild } from '@angular/router';
import { Observable } from 'rxjs';
import { map, take } from 'rxjs/operators';
import { RunboxWebmailAPI } from './rbwebmail';

@Injectable()
export class RMMAuthGuardService implements CanActivate, CanActivateChild {

urlBeforeLogin = '';
wasLoggedIn = false;
currentMe;

currentMe;

constructor(
public http: HttpClient,
public router: Router
public router: Router,
private rmmapi : RunboxWebmailAPI,
) {

}

checkLogin(): Promise<boolean | UrlTree> {
return new Promise<boolean | UrlTree>((resolve, _reject) => {
this.isLoggedIn().pipe(take(1)).subscribe(
success => {
if (!success) {
this.redirectToLogin();
resolve(this.router.parseUrl('/login'));
} else {
resolve(success);
}
resolve(success);
},
error => {
if (error.status === 403) {
Expand All @@ -62,9 +65,12 @@ export class RMMAuthGuardService implements CanActivate, CanActivateChild {
isLoggedIn(): Observable<boolean | UrlTree> {
return this.http.get('/rest/v1/me').pipe(
map((res: any) => {
this.wasLoggedIn = res && res.status === 'success';
if (res && res.result) {
this.currentMe = res.result;
if (res && res.status === 'success') {
this.wasLoggedIn = true;
const me = res.result;
// ugly? we could subscribe to it..
this.currentMe = me;
this.rmmapi.setRunboxMe(me);
}
return this.checkStatus();
}),
Expand All @@ -76,7 +82,7 @@ export class RMMAuthGuardService implements CanActivate, CanActivateChild {
if (this.currentMe && this.urlBeforeLogin
&& this.currentMe.account_status === 'expired'
&& !this.urlBeforeLogin.startsWith('/account')) {
console.log('Before login ' + this.urlBeforeLogin);
console.log('Before login ' + this.urlBeforeLogin);
return this.router.parseUrl('/account/subscriptions');
} else {
return this.wasLoggedIn;
Expand Down Expand Up @@ -115,6 +121,8 @@ export class RMMAuthGuardService implements CanActivate, CanActivateChild {
this.urlBeforeLogin = '/';
}
console.log('Will navigate back to ', this.urlBeforeLogin);
this.router.navigate(['login']);
if(this.wasLoggedIn) {
this.router.navigate(['login']);
}
}
}
23 changes: 8 additions & 15 deletions src/app/xapian/searchservice.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,12 @@ describe('SearchService', () => {
httpMock = TestBed.inject(HttpTestingController as Type<HttpTestingController>);
}));

it('should load searchservice, but no local index', async () => {
xit('should load searchservice, but no local index', async () => {
const searchService = TestBed.inject(SearchService);
await xapianLoadedSubject.toPromise();

let req = httpMock.expectOne(`/rest/v1/me`);
req.flush( { result: {
uid: 555
} as RunboxMe
});
req = httpMock.expectOne('/rest/v1/email_folder/list');
let req = httpMock.expectOne('/rest/v1/email_folder/list');
req.flush(listEmailFoldersResponse);
req = httpMock.expectOne('/rest/v1/last_on');
req.flush({'status': 'success'});

expect(await searchService.initSubject.toPromise()).toBeFalsy();
expect(searchService.localSearchActivated).toBeFalsy();
Expand Down Expand Up @@ -241,13 +234,13 @@ describe('SearchService', () => {
IDBFS.dbs = {};

const searchService = TestBed.inject(SearchService);
let req = httpMock.expectOne(`/rest/v1/me`);
req.flush( { result: {
uid: testuserid
} as RunboxMe
});
// let req = httpMock.expectOne(`/rest/v1/me`);
// req.flush( { result: {
// uid: testuserid
// } as RunboxMe
// });

req = httpMock.expectOne('/rest/v1/email_folder/list');
let req = httpMock.expectOne('/rest/v1/email_folder/list');
req.flush(listEmailFoldersResponse);


Expand Down

0 comments on commit 5317eb5

Please sign in to comment.