Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('TwoFactorAuthComponent', () => {
const event = {
preventDefault: () => {},
};
instance.sendCode(event);
await instance.sendCode(event);

expect(instance.codeSent).toBe(true);
});
Expand Down Expand Up @@ -170,4 +170,30 @@ describe('TwoFactorAuthComponent', () => {

expect(codeContainer.length).toBe(0);
});

it('should retrieve all the methods after a method has been deleted', async () => {
const { instance } = await shallow.render();

instance.methods = [
{ methodId: 'email', method: 'email', value: 'janedoe@example.com' },
];
instance.selectedMethodToDelete = {
methodId: 'email',
method: 'email',
value: 'janedoe@example.com',
};

spyOn(mockApiService.idpuser, 'disableTwoFactor').and.returnValue(
Promise.resolve(),
);
spyOn(mockApiService.idpuser, 'getTwoFactorMethods').and.returnValue(
Promise.resolve([
{ methodId: 'sms', method: 'sms', value: '(123) 456-7890' },
]),
);

await instance.submitRemoveMethod();

expect(mockApiService.idpuser.getTwoFactorMethods).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
UntypedFormGroup,
Validators,
} from '@angular/forms';
import { AccountService } from '@shared/services/account/account.service';
import { ApiService } from '@shared/services/api/api.service';
import { MessageService } from '@shared/services/message/message.service';

interface Method {
methodId: string;
Expand Down Expand Up @@ -35,6 +35,7 @@ export class TwoFactorAuthComponent implements OnInit {
constructor(
private fb: UntypedFormBuilder,
private api: ApiService,
private message: MessageService,
) {
this.form = fb.group({
code: ['', Validators.required],
Expand Down Expand Up @@ -115,7 +116,7 @@ export class TwoFactorAuthComponent implements OnInit {
this.selectedMethodToDelete.methodId,
);
} else {
this.api.idpuser.sendEnableCode(
await this.api.idpuser.sendEnableCode(
this.method,
this.form.get('contactInfo').value,
);
Expand Down Expand Up @@ -171,9 +172,7 @@ export class TwoFactorAuthComponent implements OnInit {
);
} catch (error) {
} finally {
this.methods = this.methods.filter(
(m) => m.methodId !== this.selectedMethodToDelete.methodId,
);
this.methods = await this.api.idpuser.getTwoFactorMethods();
this.selectedMethodToDelete = null;
this.codeSent = false;
this.method = null;
Expand Down
42 changes: 34 additions & 8 deletions src/app/shared/services/api/idpuser.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,55 @@ export class IdPuser extends BaseRepo {

public sendEnableCode(method: string, value: string) {
return firstValueFrom(
this.httpV2.post('/v2/idpuser/send-enable-code', { method, value }),
this.httpV2.post(
'/v2/idpuser/send-enable-code',
{ method, value },
undefined,
{
responseType: 'text',
},
),
);
}

public enableTwoFactor(method: string, value: string, code: string) {
return firstValueFrom(
this.httpV2.post('/v2/idpuser/enable-two-factor', {
method,
value,
code,
}),
this.httpV2.post(
'/v2/idpuser/enable-two-factor',
{
method,
value,
code,
},
undefined,
{
responseType: 'text',
},
),
);
}

public sendDisableCode(methodId: string) {
return firstValueFrom(
this.httpV2.post('/v2/idpuser/send-disable-code', { methodId }),
this.httpV2.post(
'/v2/idpuser/send-disable-code',
{ methodId },
undefined,
{ responseType: 'text' },
),
);
}

public disableTwoFactor(methodId: string, code: string) {
return firstValueFrom(
this.httpV2.post('/v2/idpuser/disable-two-factor', { code, methodId }),
this.httpV2.post(
'/v2/idpuser/disable-two-factor',
{ code, methodId },
undefined,
{
responseType: 'text',
},
),
);
}
}
21 changes: 21 additions & 0 deletions src/app/shared/services/http-v2/http-v2.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,25 @@ describe('HttpV2Service', () => {
'https://local.permanent.org/api/v2/health',
);
});

it('should correctly handle responseType: text', (done) => {
service
.post('/api/v2/health', {}, undefined, {
responseType: 'text',
})
.toPromise()
.then((response) => {
expect(typeof response[0]).toBe('string');
expect(response[0]).toBe('OK');
done();
})
.catch(done.fail);

const request = httpTestingController.expectOne(apiUrl('/api/v2/health'));

expect(request.request.method).toBe('POST');
expect(request.request.headers.get('Request-Version')).toBe('2');

request.flush('OK', { status: 200, statusText: 'OK' });
});
});
24 changes: 20 additions & 4 deletions src/app/shared/services/http-v2/http-v2.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,20 @@ const AUTH_KEY = 'AUTH_TOKEN';

type HttpMethod = 'post' | 'get' | 'put' | 'delete';
type ResponseClass<T> = new (data: any) => T;
type ResponseType = 'json' | 'text';

interface RequestOptions {
csrf?: boolean;
authToken?: boolean;
useStelaDomain?: boolean;
responseType?: ResponseType;
}

const defaultOptions: RequestOptions = {
csrf: false,
authToken: true,
useStelaDomain: true,
responseType: 'json',
};

export function getFirst<T>(observable: Observable<T[]>): Observable<T> {
Expand Down Expand Up @@ -187,21 +190,29 @@ export class HttpV2Service {
method: HttpMethod,
options: RequestOptions,
): Observable<unknown> {
const requestOptions: Object = {
...this.getHeaders(options),
responseType: options.responseType,
};
if (method === 'put') {
return this.http.put(url, data, this.getHeaders(options));
return this.http.put(url, data, requestOptions);
}
return this.http.post(url, data, this.getHeaders(options));
return this.http.post(url, data, requestOptions);
}

protected getObservableWithNoBody(
url: string,
method: HttpMethod,
options: RequestOptions,
): Observable<unknown> {
const requestOptions: Object = {
...this.getHeaders(options),
responseType: options.responseType,
};
if (method === 'delete') {
return this.http.delete(url, this.getHeaders(options));
return this.http.delete(url, requestOptions);
}
return this.http.get(url, this.getHeaders(options));
return this.http.get(url, requestOptions);
}

protected getObservable(
Expand Down Expand Up @@ -233,8 +244,13 @@ export class HttpV2Service {
options: RequestOptions = defaultOptions,
): Observable<T[]> {
const observable = this.getObservable(endpoint, data, method, options);

return observable.pipe(
map((response: Object | Array<Object>) => {
if (options.responseType === 'text') {
return [response as unknown as T];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a legitimate question we should probably think about... if the resposneType is text, we should always be returning a string instead of an object so response should just be able to be cast to string here. Of course, this function is expecting an observable of type T[]. I wonder if there's anyway to force T to be string when responseType is set to text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. Maybe this could be a separate ticket? I have tried doing just that but I got many errors in different files unrelated to the ticket

}

if (Array.isArray(response)) {
return response.map((obj) => {
if (responseClass) {
Expand Down