-
Notifications
You must be signed in to change notification settings - Fork 34
fix: handle invalid oidc session date after jwt is expired #1939
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.
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
base: master
Are you sure you want to change the base?
Conversation
…/frontend into datasets-filter-conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @abdimo101 - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -179,6 +220,229 @@ export class DatasetsFilterComponent implements OnInit, OnDestroy { | |||
this.store.dispatch(fetchFacetCountsAction()); | |||
} | |||
|
|||
trackByCondition(index: number, conditionConfig: ConditionConfig): string { | |||
const condition = conditionConfig.condition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring
)
const condition = conditionConfig.condition; | |
const {condition} = conditionConfig; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
this.asyncPipe.transform(this.conditionConfigs$) || []; | ||
const updatedConditions = [...currentConditions]; | ||
updatedConditions[index] = { ...updatedConditions[index], enabled }; | ||
const condition = updatedConditions[index].condition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring
)
const condition = updatedConditions[index].condition; | |
const {condition} = updatedConditions[index]; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
@@ -39,7 +39,8 @@ export class AuthService { | |||
this.token.id = this.load("id"); | |||
this.token.user = JSON.parse(this.load("user") || null); | |||
this.token.userId = this.load("userId"); | |||
this.token.created = new Date(this.load("created")); | |||
const date = new Date(this.load("created")); | |||
this.token.created = !isNaN(date.getTime()) ? date : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Invert ternary operator to remove negation (invert-ternary
)
this.token.created = !isNaN(date.getTime()) ? date : null; | |
this.token.created = isNaN(date.getTime()) ? null : date; |
Explanation
Negated conditions are more difficult to read than positive ones, so it is bestto avoid them where we can. By inverting the ternary condition and swapping the
expressions we can simplify the code.
Description
Fix authentication issues and infinite loading after logging in with OIDC and returning with an expired JWT.
Motivation
When logging in with OIDC and returning to the site after the JWT expires, the site would attempt to fetch user data and dataset table get stuck in an infinite loading state. This happened because the invalid
created
property in OIDC session caused theisTokenExpired
check to fail.Fixes:
created
property to prevent unnecessary API calls and ensures authentication cookies are cleared when the JWT has expired.Changes:
Tests included
Documentation
official documentation info
If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included
Backend version
Summary by Sourcery
Handle invalid OIDC session dates to fix authentication loops after expired JWTs and overhaul the dataset filter component to introduce a full-featured scientific conditions UI with corresponding test coverage
Bug Fixes:
Enhancements:
Build:
Tests: