Skip to content

fix: update processLoginCallbacks to use sessionDuration instead of r… #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

fix: update processLoginCallbacks to use sessionDuration instead of r… #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions adminforth/modules/restApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
this.adminforth = adminforth;
}

async processLoginCallbacks(adminUser: AdminUser, toReturn: { redirectTo?: string, allowedLogin:boolean, error?: string }, response: any, extra: HttpExtra, rememberMeDays?: number) {
async processLoginCallbacks(adminUser: AdminUser, toReturn: { redirectTo?: string, allowedLogin:boolean, error?: string }, response: any, extra: HttpExtra, sessionDuration?: number) {
const beforeLoginConfirmation = this.adminforth.config.auth.beforeLoginConfirmation as (BeforeLoginConfirmationFunction[] | undefined);

for (const hook of listify(beforeLoginConfirmation)) {
Expand All @@ -132,7 +132,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
response,
adminforth: this.adminforth,
extra,
rememberMeDays
sessionDuration,
});

if (resp?.body?.redirectTo || resp?.error) {
Expand Down
5 changes: 3 additions & 2 deletions adminforth/types/Back.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,9 @@ export interface IAdminForthRestAPI {
* @param adminUser - plugin/af pases current adminUser
* @param toReturn - this is an object which will get status of login process. If at least one callback returns error or redirectTo, login process will be stopped (future callbacks will not be called).
* @param response - http response object
* @param sessionDuration - duration of session in days
Copy link
Contributor

Choose a reason for hiding this comment

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

@SerVitasik I thought we agreed on using :"compact string literals" (Go/Prometheus-style) like '1m', '2h' '3d', '1m' etc. As I said short durations might be pretty preferred for fin sector so lets rework it now to not migrate in future.

If it is number then variable name should indicate what measure it had (.e.g. sessionDurationDays) but it is better have time-string syntax

*/
processLoginCallbacks(adminUser: AdminUser, toReturn: { redirectTo?: string, allowedLogin: boolean, error?: string }, response: any, extra: HttpExtra): Promise<void>;
processLoginCallbacks(adminUser: AdminUser, toReturn: { redirectTo?: string, allowedLogin: boolean, error?: string }, response: any, extra: HttpExtra, sessionDuration?: number): Promise<void>;
}

export interface IAdminForth {
Expand Down Expand Up @@ -604,7 +605,7 @@ export type BeforeLoginConfirmationFunction = (params?: {
response: IAdminForthHttpResponse,
adminforth: IAdminForth,
extra?: HttpExtra,
rememberMeDays?: number,
sessionDuration?: number,
}) => Promise<{
error?: string,
body: {
Expand Down