-
Notifications
You must be signed in to change notification settings - Fork 1
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
Account Service SIWF v2 URI Validation #669
Conversation
… in account configmap
expect(accountServiceConfig.siwfV2Url).toStrictEqual(ALL_ENV.SIWF_V2_URL); | ||
}); | ||
|
||
it.each([['https://example.com/login'], ['example://login'], ['localhost'], ['localhost:3030/login/path']])( |
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.
nice!
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.
LGTM 👍
@@ -41,12 +41,6 @@ describe('Account API Config', () => { | |||
it('invalid api timeout limit should fail', async () => shouldFailBadValues(ALL_ENV, 'API_TIMEOUT_MS', [0])); | |||
|
|||
it('invalid url for SIWF_V2_URL', async () => shouldFailBadValues(ALL_ENV, 'SIWF_V2_URL', ['sdfdsf'])); | |||
|
|||
it('invalid url SIWF_V2_DOMAIN', async () => | |||
shouldFailBadValues(ALL_ENV, 'SIWF_V2_DOMAIN', ['https://www.example.com'])); |
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.
These values are valid now.
@@ -18,7 +18,8 @@ data: | |||
HEALTH_CHECK_SUCCESS_THRESHOLD: {{ .Values.account.env.HEALTH_CHECK_SUCCESS_THRESHOLD | quote }} | |||
CAPACITY_LIMIT: {{ .Values.account.env.CAPACITY_LIMIT | quote }} | |||
SIWF_URL: {{ .Values.account.env.SIWF_URL | quote }} | |||
SIWF_DOMAIN: {{ .Values.account.env.SIWF_DOMAIN | quote }} | |||
SIWF_V2_URL: {{ .Values.account.env.SIWF_V2_URL | quote }} |
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.
@saraswatpuneet This value is optional, is it okay to put it or will it cause errors if it is not set?
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 from my view.
- Reviewed code
Problem
Updates to support siwf v2.1.0
Closes #640
Solution
SIWF_V2_DOMAIN
toSIWF_V2_URI_VALIDATION
and follow on config changes, tests, etc.loginMsgDomain
tologinMsgURIValidation
ENVIRONMENT.md