Deployment auth SIP by itowlson · Pull Request #795 · spinframework/spin

itowlson · 2022-09-30T00:45:56Z

WIP but getting discussion under way!

bacongobbler

Looking great! Thanks for taking the time to write this up.

docs/content/sips/007-deployment-auth.md

bacongobbler · 2022-09-30T17:28:48Z

docs/content/sips/007-deployment-auth.md

+
+A user may log into multiple Hippo servers.  The (URL, token) pairs for each will be stored.  That is, logging into one server does not log you out of others.
+
+> QUESTION: Or should you only be logged into one at a time?


To simplify the spin deploy experience, being logged into one system at a time may be best for now. Especially because this proposed idea raises a bunch of follow-up questions listed further below.

Once we design a system where we can support multiple "login profiles" (spin configure has been thrown around as an idea), then we should revisit this idea.

docs/content/sips/007-deployment-auth.md

bacongobbler · 2022-09-30T18:04:59Z

docs/content/sips/007-deployment-auth.md

+> * Con: the user may already have stuff on their clipboard
+> * Con: again, documentation needs to consider differences across OSes
+
+(For both of these questions, is there prior art to which we can refer?)


Commands like az login and gh auth login would be good places to reference.

https://cli.github.com/manual/gh_auth_login

https://learn.microsoft.com/en-us/cli/azure/authenticate-azure-cli

docs/content/sips/007-deployment-auth.md

lann · 2022-10-03T15:18:47Z

docs/content/sips/007-deployment-auth.md

+
+We propose to poll for two minutes. If the user has not entered the code by then, `spin login` fails and exits.
+
+Errors do _not_ cause polling to terminate. We do not want a transient network error or server hiccup to block login. That said, if we can identify server responses that are necessarily fatal, we can in future terminate early on those.


They should at least print warning(s) or the user may be left wondering.

docs/content/sips/007-deployment-auth.md

itowlson · 2022-10-03T20:03:19Z

Thanks folks for all the feedback. It sounds like the model we want to espouse is:

spin login both selects the deployment server and records credentials for it.
- Even for username-password servers.
- You can only be logged into one server at a time - logging out, or logging into another server, not only deselects it but erases the stored credentials.
- We need a solution for folks who often switch between multiple deployment servers - one possibility is to have them manage file locations.
spin deploy always deploys to the target specified in the last login (and we remove all deployment server options).
- Possible exception for CI environments?
- If you're not logged in then it either fails entirely, or hands you off to the login flow (from Fisher's example it looks like if you're not logged in it just fails, but if your credentials fail it hands you off to the login flow?). But in either case there is a clear handoff - we do not entangle auth and deployment.
- We are willing to add an extra step to the spin new -> spin build -> spin deploy pipeline because it allows us to vastly simplify deploy.

Is that correct?

bacongobbler · 2022-10-03T20:36:41Z

Is that correct?

Yes. Your assessment matches my own mental model.

The last step seems like it is the most impactful to the UX.

You can only be logged into one server at a time - logging out, or logging into another server, not only deselects it but erases the stored credentials.

In order to minimize impact to the spin CLI, this makes sense for now. But we should circle back on this one ASAP to see if we can add multiple login profile support as this will be a big QoL improvement to the UX.

mikkelhegn · 2022-10-03T20:42:49Z

spin deploy always deploys to the target specified in the last login (and we remove all deployment server options)
Without a --target (or similar) option, I would probably at any given time deploy to the wrong target, so is the endpoint you are signed in to stored somewhere? I'm asking as a prompt (e.g. Starship) could show that info if that's the case.

itowlson · 2022-10-04T01:38:53Z

@bacongobbler Hope this now accurately reflects our discussions!

itowlson · 2022-10-05T02:27:28Z

@bacongobbler one thing not covered here... there probably needs to be a way to tell what you're logged into... I'm reluctant to have a top level command for that though... would something like spin login --status make sense?

bacongobbler · 2022-10-05T03:24:37Z

Yeah that makes sense to me!

bacongobbler · 2022-10-05T19:34:59Z

@itowlson see #794 (comment) - spin login --status has now been implemented.

bacongobbler · 2022-10-06T21:28:40Z

docs/content/sips/007-deployment-auth.md

+At the moment, Spin doesn't know whether a URL uses device flow or username-password, so `spin login` will prompt.  If we can identify particular URLs or families of URLs whose auth mode is known, those can skip this step.
+
+```
+How would you like to authenticate Spin?  [Username and password|Log in with a web browser]


This is a good way to handle this issue for the time being.

This is how it looks as written in #794:

Suggested change

How would you like to authenticate Spin? [Username and password|Log in with a web browser]

What authentication method does this server support?

1. Sign in with GitHub

2. Sign in with a username and password

Enter a number:

bacongobbler · 2022-10-07T15:46:53Z

docs/content/sips/007-deployment-auth.md

+At the moment, Spin doesn't know whether a URL uses device flow or username-password, so `spin login` will prompt.  If we can identify particular URLs or families of URLs whose auth mode is known, those can skip this step.
+
+```
+How would you like to authenticate Spin?  [Username and password|Log in with a web browser]


This is how it looks as written in #794:

Suggested change

How would you like to authenticate Spin? [Username and password|Log in with a web browser]

What authentication method does this server support?

1. Sign in with GitHub

2. Sign in with a username and password

Enter a number:

itowlson · 2022-10-11T03:39:58Z

@bacongobbler thanks! Updated the sign-in method prompt; I also added a note about needing a flag to suppress it (which can be hidden).

bacongobbler · 2022-10-11T21:40:18Z

docs/content/sips/007-deployment-auth.md

+Enter a number:
+```
+
+Programmatic consumers need a way to override this prompt, e.g. `--method=[username|github]`.


I think this would be a good option. We can also check for the presence of --check-device-code to determine the auth mode, since the presence of that flag automatically short-circuits it to be Github auth.

Yep, and --get-device-code too of course. I'll add this note and merge. Thanks!

Signed-off-by: itowlson <ivan.towlson@fermyon.com>

itowlson requested review from bacongobbler and radu-matei September 30, 2022 00:45

itowlson force-pushed the sip-device-flow-auth branch from a9fd9f5 to 2f28448 Compare September 30, 2022 02:20

itowlson marked this pull request as ready for review September 30, 2022 02:52

itowlson force-pushed the sip-device-flow-auth branch from 2f28448 to 1009676 Compare September 30, 2022 03:51

bacongobbler requested changes Sep 30, 2022

View reviewed changes

radu-matei added this to the Spin v0.6.0 milestone Oct 3, 2022

lann reviewed Oct 3, 2022

View reviewed changes

docs/content/sips/007-deployment-auth.md Outdated Show resolved Hide resolved

lann reviewed Oct 3, 2022

View reviewed changes

docs/content/sips/007-deployment-auth.md Outdated Show resolved Hide resolved

lann reviewed Oct 3, 2022

View reviewed changes

docs/content/sips/007-deployment-auth.md Outdated Show resolved Hide resolved

lann reviewed Oct 3, 2022

View reviewed changes

docs/content/sips/007-deployment-auth.md Outdated Show resolved Hide resolved

itowlson requested review from bacongobbler and lann October 4, 2022 01:38

bacongobbler reviewed Oct 6, 2022

View reviewed changes

bacongobbler requested changes Oct 7, 2022

View reviewed changes

itowlson force-pushed the sip-device-flow-auth branch from e735a4e to 74ba572 Compare October 11, 2022 21:22

bacongobbler approved these changes Oct 11, 2022

View reviewed changes

itowlson force-pushed the sip-device-flow-auth branch from 74ba572 to fe4c708 Compare October 11, 2022 22:40

Deployment auth SIP

1b096de

Signed-off-by: itowlson <ivan.towlson@fermyon.com>

itowlson force-pushed the sip-device-flow-auth branch from fe4c708 to 1b096de Compare October 11, 2022 22:41

itowlson merged commit b6cc918 into spinframework:main Oct 11, 2022


		A user may log into multiple Hippo servers. The (URL, token) pairs for each will be stored. That is, logging into one server does not log you out of others.

		> QUESTION: Or should you only be logged into one at a time?


		We propose to poll for two minutes. If the user has not entered the code by then, `spin login` fails and exits.

		Errors do _not_ cause polling to terminate. We do not want a transient network error or server hiccup to block login. That said, if we can identify server responses that are necessarily fatal, we can in future terminate early on those.

-How would you like to authenticate Spin?  [Username and password|Log in with a web browser]
+What authentication method does this server support?
+. Sign in with GitHub
+. Sign in with a username and password
+Enter a number:

Conversation

itowlson commented Sep 30, 2022 • edited by lann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bacongobbler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itowlson commented Oct 3, 2022

Uh oh!

bacongobbler commented Oct 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikkelhegn commented Oct 3, 2022

Uh oh!

itowlson commented Oct 4, 2022

Uh oh!

itowlson commented Oct 5, 2022

Uh oh!

bacongobbler commented Oct 5, 2022

Uh oh!

bacongobbler commented Oct 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itowlson commented Oct 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

itowlson commented Sep 30, 2022 •

edited by lann

Loading

bacongobbler commented Oct 3, 2022 •

edited

Loading