Skip to content
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

Added custom assertionConsumerServiceURL to handle wrong redirects #200

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

giordanocardillo
Copy link

Hello,

I was configuring your CLI tool for my environment but i needed to have a custom assertionConsumerServiceURL inside my SAML request, so I added it and now the post SAML redirect is working correctly. I'd like to share it with you.

@divyenduz
Copy link

We also have the same use case. Is it possible to merge and release this one?

Copy link

@Abdelhamid-Ibrahim Abdelhamid-Ibrahim left a comment

Choose a reason for hiding this comment

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

Great work 🎖️ I left some comments to make sure that it could be loaded from env vars and also be saved to config file as an input

Comment on lines +24 to +29
{
name: "assertionConsumerServiceURL",
message: "Azure Assertion Consumer ID URI:",
validate: (input): boolean => !!input,
default: profile && profile.azure_assertion_consumer_service_url,
},

Choose a reason for hiding this comment

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

For the same file I think you might add it here as well

await awsConfig.setProfileConfigValuesAsync(profileName, {
    azure_tenant_id: answers.tenantId as string,
    azure_app_id_uri: answers.appIdUri as string,
    azure_default_username: answers.username as string,
    azure_default_role_arn: answers.defaultRoleArn as string,
    azure_assertion_consumer_service_url: answers.assertionConsumerServiceURL as string,
    azure_default_duration_hours: answers.defaultDurationHours as string,
    azure_default_remember_me: (answers.rememberMe as string) === "true",
  });

Comment on lines +411 to +413
if (profile.azure_assertion_consumer_service_url) {
assertionConsumerServiceURL = profile.azure_assertion_consumer_service_url
}

Choose a reason for hiding this comment

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

you might as well add it to the section of loading it from env vars here

  _loadProfileFromEnv(): { [key: string]: string } {
    const env: { [key: string]: string } = {};
    const options = [
      "azure_tenant_id",
      "azure_app_id_uri",
      "azure_default_username",
      "azure_default_password",
      "azure_default_role_arn",
      "azure_default_duration_hours",
      "azure_assertion_consumer_service_url"
    ];

@atrull
Copy link

atrull commented Apr 8, 2024

c'mon let's make this mergable ? ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants