-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3260 - CAS Integration 3A - Create new Supplier and Site - Refactor CAS authentication #3822
Conversation
...es/backend/libs/integrations/src/cas/_tests_/unit/cas.service.getSupplierInfoFromCAS.spec.ts
Dismissed
Show dismissed
Hide dismissed
...cas-supplier/cas-evaluation-result-processor/cas-active-supplier-and-site-found-processor.ts
Show resolved
Hide resolved
...services/cas-supplier/cas-evaluation-result-processor/cas-active-supplier-found-processor.ts
Show resolved
Hide resolved
/** | ||
* Cache the CAS token to be reused. | ||
*/ | ||
export class CachedCASAuthDetails { |
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.
I'm not sure if this class should be in a model file. I'd rather have it in cas.service.ts
or have a new file for CachedCASAuthDetails
.
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.
Move to a different file.
sources/packages/backend/libs/integrations/src/cas/models/cas-service.model.ts
Outdated
Show resolved
Hide resolved
* Default amount of time to a token be expired and be | ||
* considered candidate to be renewed. | ||
*/ | ||
const CAS_TOKEN_RENEWAL_SECONDS = 60; |
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.
in system configuration constants ?
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.
I do not see a value in moving this. It is a private const only used to create some difference for the actual expiration time.
...processors/schedulers/cas-integration/_tests_/cas-supplier-integration.scheduler.e2e-spec.ts
Outdated
Show resolved
Hide resolved
Thanks for making this refactor to simplify the CAS logon call. Please have a look at the comments. |
Quality Gate passedIssues Measures |
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.
Thanks for making the changes. Looks Good. 👍
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.
Looks good! Great job!
As agreed in the previous PR, the CAS authentication is now encapsulated inside the CAS API service and the auth token no longer needs to be passed to every method.
The current CAS token expiration is set to 1 hour and the token will be considered "about to expire" one minute before it.