common-fate / granted

The easiest way to access your cloud.
https://granted.dev
MIT License
955 stars 90 forks source link

automatically add `--auto-login` flag to `credential_process` keys when using `granted sso populate` #683

Closed admackin closed 2 weeks ago

admackin commented 2 weeks ago

The Credential Process feature is great, and so is the --auto-login flag. However this field is not set when using granted sso populate leading to authentication failures when using auto-generated profiles.

I think the flag should be added automatically (is there any disadvantage?), but it might make sense to allow users to opt out

chrnorm commented 2 weeks ago

Thanks for raising this @admackin. Some context: the disadvantage to adding --auto-login by default is that if Granted is run in a terminal-only environment (say, on an EC2 instance), --auto-login causes the credential process to hang without printing any output. This is due to a limitation in the AWS CLI, which doesn't print any credential process stderr/stdout until after the credential process finishes executing.

As a workaround, you can currently set a global variable to always automatically login:

granted settings set --setting=CredentialProcessAutoLogin --value true

I am thinking though we should improve our default behaviour to something like the following:

  1. When the credential process is executed, try and open the browser
  2. If opening the browser fails (as should be the case in a terminal-only environment), return an error telling the user to run granted sso login

We will need to test that the terminal-only detection works properly though. Any ideas here are most welcome!

admackin commented 2 weeks ago

Ah yes that all makes sense, thanks @chrnorm ! Is that setting documented anywhere? It would probably cover my use case. I searched on the site and it didn't come up but I may have missed it.

It sounds like until auto login has described behaviour my proposal would only work as an opt in flag to sso populate then? Your idea sounds much better but I don't have any special expertise to offer on it.

Up to you whether to close this issue

admackin commented 2 weeks ago

Further to above: can I suggest you document the above setting here? I had a look at opening a PR to do that but it doesn't look like the docs site is open source?

chrnorm commented 2 weeks ago

Thanks @admackin, I've added a section to our docs here: https://docs.commonfate.io/granted/recipes/credential-process#global-auto-login.

I've also open sourced our docs here! https://github.com/common-fate/docs. We migrated from Docusaurus to Mintlify a while ago and our initial Mintlify docs repo was closed source. This is now fixed and docs contributions are most welcome.