-
Notifications
You must be signed in to change notification settings - Fork 26
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
[#158907283] Replace the old spid-testenv-identityserver with the new spid-testenv2 #287
Conversation
…he new spid-testenv2
Affected stories
Generated by 🚫 dangerJS |
@@ -90,7 +90,7 @@ container.register({ | |||
const SAML_CALLBACK_URL = | |||
process.env.SAML_CALLBACK_URL || | |||
"http://italia-backend/assertionConsumerService"; | |||
const SAML_ISSUER = process.env.SAML_ISSUER || "http://italia-backend"; | |||
const SAML_ISSUER = process.env.SAML_ISSUER || "http://italiabackend.it"; |
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.
why this one ?
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.
because in the new testenv2 the issuer has to be a valid URL
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.
is there any reason on why this differs from the default saml issuer (https://spid.agid.gov.it/cd) ?
I mean, cant we just put https://spid.agid.gov.it/cd into the sp_metatadata.xml of the test spid IDP ?
32ea3d5
to
533fe91
Compare
src/container.ts
Outdated
@@ -221,7 +224,7 @@ container.register({ | |||
function createSimpleRedisClient(): redis.RedisClient { | |||
const redisUrl = process.env.REDIS_URL || "redis://redis"; | |||
log.info("Creating SIMPLE redis client", { url: redisUrl }); | |||
return redis.createClient(); | |||
return redis.createClient(redisUrl); |
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.
may you please open a new PR for this one ? (or a pivotal bug)
@lussoluca @lmorelli986 now that we have deployed the spid-testenv2 idp, can we start merging the integration without autologin? we need it for the Apple approval process, we can provide them with a username and a password |
In other words, this PR should add just one more entry to the spidStrategy.ts with the test IDP info. |
The autologin user is configured as an environment variable. We can set that variable in the CI environment to perform tests.
At the moment we are blocked by italia/spid-testenv2#118