Skip to content

Commit

Permalink
refactor(major): loadSession throw error instead of throw in next
Browse files Browse the repository at this point in the history
  • Loading branch information
@akku-aakash
akku-aakash committed Dec 18, 2024
1 parent 1ec4856 commit 203ff61
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 58 deletions.
8 changes: 3 additions & 5 deletions oidc-consumer/DOCUMENTATION.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Middlewares and utilities for OIDC
* [.parseCallback()](#OidcConsumer+parseCallback)
* [.authCallback(request, response, next, queryParams, [httpOptions])](#OidcConsumer+authCallback)
* [.refresh(token, scope, [httpOptions])](#OidcConsumer+refresh) ⇒
* [.loadSession(request, response, next, retryOnFailure)](#OidcConsumer+loadSession)
* [.loadSession(session, retryOnFailure)](#OidcConsumer+loadSession)
* [.revoke(token, token_type, [httpOptions])](#OidcConsumer+revoke) ⇒

<a name="OidcConsumer+scope"></a>
Expand Down Expand Up @@ -140,7 +140,7 @@ refresh stale or expired tokens based on a given scope

<a name="OidcConsumer+loadSession"></a>

### oidcConsumer.loadSession(request, response, next, retryOnFailure)
### oidcConsumer.loadSession(session, retryOnFailure)
Load the session in request

**Kind**: instance method of [<code>OidcConsumer</code>](#OidcConsumer)
Expand All @@ -151,9 +151,7 @@ Load the session in request

| Param | Default | Description |
| --- | --- | --- |
| request | | Express request object |
| response | | Express response object |
| next | | Express next object |
| session | | Express request session |
| retryOnFailure | <code>true</code> | Flag to throw error if found or recursively call itself |

<a name="OidcConsumer+revoke"></a>
Expand Down
59 changes: 25 additions & 34 deletions oidc-consumer/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class OidcConsumer {
async authRedirect(request: Request, response: Response, next: NextFunction, queryParams?: Object) {
const { redirectUri: destination } = request.query;

if (!destination) return next(new Error("MISSING_DESTINATION"));
if (!destination) return next(new Error("MISSING_DESTINATION"));

if (!this.isRedirectUriAllowed(String(destination), this.allowedRedirectURIs)) {
request.session.destroy((error) => {
Expand All @@ -131,10 +131,14 @@ class OidcConsumer {
});

request.session.save(async () => {
await this.loadSession(request, response, next, false);
response.redirect(authorizationURI);
try {
await this.loadSession(request.session as ICustomSession, false);
response.redirect(authorizationURI);
} catch (error) {
console.log("error in loading session @authRedirect", error);
return next(new Error(error));
}
});

}

isRedirectUriAllowed(url: string, allowedUris: any) {
Expand Down Expand Up @@ -196,13 +200,16 @@ class OidcConsumer {

if (!(request.session as ICustomSession).state) {
console.log("Reloading session because no state in session.");
await this.loadSession(request, response, next);
try {
await this.loadSession(request.session as ICustomSession);
} catch (error) {
console.log("error in loading session @authCallback", error);
return next(new Error(error));
}
}

const sessionState = (request.session as ICustomSession).state;
if (!sessionState) return;

if (state !== sessionState) return next(new Error("SECRET_MISMATCH"));
if (state !== sessionState) return next(new Error("SECRET_MISMATCH"));

const destination = (request.session as ICustomSession).redirect_uri;

Expand Down Expand Up @@ -264,36 +271,20 @@ class OidcConsumer {
* @param next - Express next object
* @param retryOnFailure - Flag to throw error if found or recursively call itself
* @throws SESSION_LOAD_FAILED if session load fails.
*/
async loadSession(request: Request, response: Response, next: NextFunction, retryOnFailure: Boolean = true, sessionRetryDelayMS: number = this.sessionRetryDelayMS) {
await new Promise<void>(resolve => {
request.session.reload((err) => {
if(err) {
console.log(err);
}
});
resolve();
*/
async loadSession(session: ICustomSession, retryOnFailure: Boolean = true, sessionRetryDelayMS: number = this.sessionRetryDelayMS) {
await new Promise<void>((resolve, reject) => {
session.reload((error) => (error ? reject("SESSION_LOAD_FAILED") : resolve()));
}).catch((error) => {
console.log(error);
console.log("error loading session from store")
console.log("Error loading session from store", error);
if (!retryOnFailure) throw error;
});

const state = (request.session as ICustomSession).state;

if (!state) {
if (retryOnFailure) {
await new Promise<void>((resolve) => {
setTimeout(async () => {
await this.loadSession(request, response, next, false);
resolve();
}, sessionRetryDelayMS);
}).catch((error) => {
console.log(error);
console.log("error in retry");
});
} else return next(new Error("SESSION_LOAD_FAILED"));
if (!session.state && retryOnFailure) {
await new Promise<void>((resolve) => setTimeout(resolve, sessionRetryDelayMS));
await this.loadSession(session, false);
}
};
}

/**
* revokes a given token for a given type
Expand Down
74 changes: 55 additions & 19 deletions oidc-consumer/test/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,34 +33,70 @@ const consumer = new OidcConsumer({
},
});

describe('Authentication Functions', () => {

describe("Authentication Functions", () => {
describe('loadSession', () => {
it('should call next() if session state is present', async () => {

it('should successfully load session if session state is present', async () => {
const req = mockReq();
const res = mockRes();
req.session = { state: 'dummy state../dist/cjs/src/index', reload: jest.fn()};

const next = sinon.spy();

await consumer.loadSession(req, res, next);

expect(next.calledOnce).toBe(false);
req.session = { state: 'valid state', reload: jest.fn((cb) => cb()) };

await consumer.loadSession(req.session, true);
expect(req.session.reload).toHaveBeenCalledTimes(1);
});

it('should throw an error if session state is not present and throwError is true', async () => {
it('should throw an error if session.reload fails (rejects)', async () => {
const req = mockReq();
const res = mockRes();
req.session = {reload: jest.fn()};
req.session = { reload: jest.fn((cb) => cb(new Error('Failed to load session')))};

try {
await consumer.loadSession(req.session, true);
expect(true).toBe(true);
} catch (error) {
expect(error).toEqual('SESSION_LOAD_FAILED');
}

expect(req.session.reload).toHaveBeenCalledTimes(2);
});

it('should throw an error if session state is missing and retryOnFailure is false', async () => {
const req = mockReq();
req.session = { reload: jest.fn((cb) => cb()) };

const next = sinon.spy();
try {
await consumer.loadSession(req.session, false);
expect(true).toBe(true);
} catch (error) {
expect(error).toEqual('SESSION_LOAD_FAILED');
}

expect(req.session.reload).toHaveBeenCalledTimes(1);
});

it('should retry once when session state is missing and retryOnFailure is true', async () => {
const req = mockReq();
req.session = { reload: jest.fn((cb) => cb()) };

await consumer.loadSession(req, res, next, true);
expect(next.calledOnce).toBe(true);
expect(next.firstCall.args[0]).toBeInstanceOf(Error);
expect(next.firstCall.args[0].message).toEqual('SESSION_LOAD_FAILED');
await consumer.loadSession(req.session, true);
expect(req.session.reload).toHaveBeenCalledTimes(2);
});

it('should throw an error after retry if session state is still missing and retryOnFailure is true', async () => {
const req = mockReq();
req.session = { reload: jest.fn((cb) => cb()) };

try {
await consumer.loadSession(req.session, true);
expect(true).toBe(true);
} catch (error) {
expect(error).toEqual('SESSION_LOAD_FAILED');
}

expect(req.session.reload).toHaveBeenCalledTimes(2);
});


});


describe('authCallback', () => {
it('should call next() if state matches session state', async () => {
Expand Down

0 comments on commit 203ff61

Please sign in to comment.