Skip to content

Commit

Permalink
feat: authts#4 reduce usage of any type
Browse files Browse the repository at this point in the history
  • Loading branch information
pamapa committed Aug 24, 2021
1 parent b220b93 commit 862b2a4
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 97 deletions.
27 changes: 16 additions & 11 deletions src/SigninRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export class SigninRequest {

public constructor({
// mandatory
url, client_id, redirect_uri, response_type, scope, authority,
url, authority, client_id, redirect_uri, response_type, scope,
// optional
data, prompt, display, max_age, ui_locales, id_token_hint, login_hint, acr_values, resource, response_mode,
request, request_uri, extraQueryParams, request_type, client_secret, extraTokenParams, skipUserInfo
Expand Down Expand Up @@ -40,29 +40,34 @@ export class SigninRequest {
throw new Error("authority");
}

const oidc = SigninRequest.isOidc(response_type);
const code = SigninRequest.isCode(response_type);
const isOidc = SigninRequest.isOidc(response_type);
const isCode = SigninRequest.isCode(response_type);

if (!response_mode) {
response_mode = SigninRequest.isCode(response_type) ? "query" : null;
response_mode = isCode ? "query" : undefined;
}

this.state = new SigninState({ nonce: oidc,
data, client_id, authority, redirect_uri,
code_verifier: code,
request_type, response_mode,
client_secret, scope, extraTokenParams, skipUserInfo });
this.state = new SigninState({
data,
request_type,
nonce: isOidc,
code_verifier: isCode,
client_id, authority, redirect_uri,
response_mode,
client_secret, scope, extraTokenParams,
skipUserInfo
});

url = UrlUtility.addQueryParam(url, "client_id", client_id);
url = UrlUtility.addQueryParam(url, "redirect_uri", redirect_uri);
url = UrlUtility.addQueryParam(url, "response_type", response_type);
url = UrlUtility.addQueryParam(url, "scope", scope);

url = UrlUtility.addQueryParam(url, "state", this.state.id);
if (oidc) {
if (this.state.nonce) {
url = UrlUtility.addQueryParam(url, "nonce", this.state.nonce);
}
if (code) {
if (this.state.code_challenge) {
url = UrlUtility.addQueryParam(url, "code_challenge", this.state.code_challenge);
url = UrlUtility.addQueryParam(url, "code_challenge_method", "S256");
}
Expand Down
2 changes: 1 addition & 1 deletion src/SigninResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export class SigninResponse {
public readonly code: string;

// updated by ResponseValidator
public state: string | undefined;
public state: any | undefined;

// updated by ResponseValidator
public error: string | undefined;
Expand Down
87 changes: 53 additions & 34 deletions src/SigninState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,54 +5,72 @@ import { Log, JoseUtil, random } from "./utils";
import { State } from "./State";

export class SigninState extends State {
public readonly nonce: any;
public readonly code_verifier: any;
public readonly code_challenge: any;
public readonly redirect_uri: any;
public readonly authority: any;
public readonly client_id: any;
public readonly response_mode: any;
public readonly client_secret: any;
public readonly scope: any;
public readonly extraTokenParams: any;
public readonly skipUserInfo: any;
// isOidc
public readonly nonce: string | undefined;

public constructor(args: any = {}) {
const {
nonce, authority, client_id,
redirect_uri, code_verifier, response_mode, client_secret,
scope, extraTokenParams, skipUserInfo
} = args;
// isCode
public readonly code_verifier: string | undefined;
public readonly code_challenge: string | undefined;

// to ensure state still matches settings
public readonly authority: string;
public readonly client_id: string;
public readonly redirect_uri: string;
public readonly scope: string;
public readonly client_secret: string | undefined;
public readonly extraTokenParams: Record<string, any> | undefined;

public readonly response_mode: string | undefined;
public readonly skipUserInfo: boolean | undefined;

public constructor(args: {
id?: string;
data?: any;
created?: number;
request_type: string;

nonce?: string | boolean;
code_verifier?: string | boolean;
authority: string;
client_id: string;
redirect_uri: string;
scope: string;
client_secret?: string;
extraTokenParams?: Record<string, any>;
response_mode?: string;
skipUserInfo?: boolean;
}) {
super(args);

if (nonce === true) {
if (args.nonce === true) {
this.nonce = random();
}
else if (nonce) {
this.nonce = nonce;
else if (args.nonce) {
this.nonce = args.nonce;
}

if (code_verifier === true) {
if (args.code_verifier === true) {
// random() produces 32 length
this.code_verifier = random() + random() + random();
}
else if (code_verifier) {
this.code_verifier = code_verifier;
else if (args.code_verifier) {
this.code_verifier = args.code_verifier;
}

if (this.code_verifier) {
const hash = JoseUtil.hashString(this.code_verifier, "SHA256");
this.code_challenge = JoseUtil.hexToBase64Url(hash);
}

this.redirect_uri = redirect_uri;
this.authority = authority;
this.client_id = client_id;
this.response_mode = response_mode;
this.client_secret = client_secret;
this.scope = scope;
this.extraTokenParams = extraTokenParams;
this.skipUserInfo = skipUserInfo;
this.authority = args.authority;
this.client_id = args.client_id;
this.redirect_uri = args.redirect_uri;
this.scope = args.scope;
this.client_secret = args.client_secret;
this.extraTokenParams = args.extraTokenParams;

this.response_mode = args.response_mode;
this.skipUserInfo = args.skipUserInfo;
}

public toStorageString() {
Expand All @@ -62,15 +80,16 @@ export class SigninState extends State {
data: this.data,
created: this.created,
request_type: this.request_type,

nonce: this.nonce,
code_verifier: this.code_verifier,
redirect_uri: this.redirect_uri,
authority: this.authority,
client_id: this.client_id,
response_mode: this.response_mode,
client_secret: this.client_secret,
redirect_uri: this.redirect_uri,
scope: this.scope,
client_secret: this.client_secret,
extraTokenParams : this.extraTokenParams,
response_mode: this.response_mode,
skipUserInfo: this.skipUserInfo
});
}
Expand Down
21 changes: 12 additions & 9 deletions src/State.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,24 @@ export class State {
public readonly id: string;
public readonly data: any;
public readonly created: number;
public readonly request_type: any;
public readonly request_type: string;

public constructor({
id, data, created, request_type
}: any = {}) {
this.id = id || random();
this.data = data;
public constructor(args: {
id?: string;
data?: any;
created?: number;
request_type: string;
}) {
this.id = args.id || random();
this.data = args.data;

if (typeof created === "number" && created > 0) {
this.created = created;
if (args.created && args.created > 0) {
this.created = args.created;
}
else {
this.created = Timer.getEpochTime();
}
this.request_type = request_type;
this.request_type = args.request_type;
}

public toStorageString() {
Expand Down
37 changes: 32 additions & 5 deletions test/unit/OidcClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,15 @@ describe("OidcClient", () => {

it("should deserialize stored state and return state and response", async () => {
// arrange
const item = new SigninState({ id: "1", nonce: "2", authority:"authority", client_id:"client", request_type:"type" }).toStorageString();
const item = new SigninState({
id: "1",
nonce: "2",
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
}).toStorageString();
jest.spyOn(subject.settings.stateStore, "get").mockImplementation(() => Promise.resolve(item));

// act
Expand Down Expand Up @@ -317,7 +325,15 @@ describe("OidcClient", () => {

it("should deserialize stored state and call validator", async () => {
// arrange
const item = new SigninState({ id: "1", nonce: "2", authority:"authority", client_id:"client" });
const item = new SigninState({
id: "1",
nonce: "2",
authority: "authority",
client_id:"client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
jest.spyOn(subject.settings.stateStore, "remove")
.mockImplementation(() => Promise.resolve(item.toStorageString()));
const validateSigninResponseMock = jest.spyOn(subject["_validator"], "validateSigninResponse")
Expand Down Expand Up @@ -509,7 +525,11 @@ describe("OidcClient", () => {

it("should call validator with state even if error in response", async () => {
// arrange
const item = new State({ id: "1", data:"bar" });
const item = new State({
id: "1",
data: "bar",
request_type: "type"
});
jest.spyOn(subject.settings.stateStore, "remove")
.mockImplementation(() => Promise.resolve(item.toStorageString()));
const validateSignoutResponse = jest.spyOn(subject["_validator"], "validateSignoutResponse")
Expand Down Expand Up @@ -568,7 +588,10 @@ describe("OidcClient", () => {

it("should deserialize stored state and call validator", async () => {
// arrange
const item = new State({ id: "1" });
const item = new State({
id: "1",
request_type: "type"
});
jest.spyOn(subject.settings.stateStore, "remove")
.mockImplementation(() => Promise.resolve(item.toStorageString()));
const validateSignoutResponse = jest.spyOn(subject["_validator"], "validateSignoutResponse")
Expand All @@ -583,7 +606,11 @@ describe("OidcClient", () => {

it("should call validator with state even if error in response", async () => {
// arrange
const item = new State({ id: "1", data:"bar" });
const item = new State({
id: "1",
data:"bar",
request_type: "type"
});
jest.spyOn(subject.settings.stateStore, "remove")
.mockImplementation(() => Promise.resolve(item.toStorageString()));
const validateSignoutResponse = jest.spyOn(subject["_validator"], "validateSignoutResponse")
Expand Down
48 changes: 42 additions & 6 deletions test/unit/ResponseValidator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,13 @@ describe("ResponseValidator", () => {

it("should filter protocol claims if OIDC", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
stubResponse.isOpenIdConnect = true;
stubResponse.profile = { a: "apple", b: "banana" };
const _filterProtocolClaimsMock = jest.spyOn(subject, "_filterProtocolClaims")
Expand All @@ -403,7 +409,13 @@ describe("ResponseValidator", () => {

it("should not filter protocol claims if not OIDC", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
stubResponse.isOpenIdConnect = false;
const _filterProtocolClaimsMock = jest.spyOn(subject, "_filterProtocolClaims")
.mockImplementation((profile) => profile);
Expand All @@ -417,7 +429,13 @@ describe("ResponseValidator", () => {

it("should load and merge user info claims when loadUserInfo configured", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
settings.loadUserInfo = true;
stubResponse.isOpenIdConnect = true;
stubResponse.profile = { a: "apple", b: "banana" };
Expand All @@ -436,7 +454,13 @@ describe("ResponseValidator", () => {

it("should not run if reqest was not openid", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
settings.loadUserInfo = true;
stubResponse.isOpenIdConnect = false;
stubResponse.profile = { a: "apple", b: "banana" };
Expand All @@ -452,7 +476,13 @@ describe("ResponseValidator", () => {

it("should not load and merge user info claims when loadUserInfo not configured", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
settings.loadUserInfo = false;
stubResponse.isOpenIdConnect = true;
stubResponse.profile = { a: "apple", b: "banana" };
Expand All @@ -468,7 +498,13 @@ describe("ResponseValidator", () => {

it("should not load user info claims if no access token", async () => {
// arrange
const state = new SigninState();
const state = new SigninState({
authority: "authority",
client_id: "client",
redirect_uri: "http://cb",
scope: "scope",
request_type: "type"
});
settings.loadUserInfo = true;
stubResponse.isOpenIdConnect = true;
stubResponse.profile = { a: "apple", b: "banana" };
Expand Down
Loading

0 comments on commit 862b2a4

Please sign in to comment.