Skip to content

set cluster connector name on signin for first cloud user#17834

Merged
JanKaczmarkiewicz merged 41 commits intomasterfrom
jan/cloud-passwordless-default-preference
Nov 14, 2022
Merged

set cluster connector name on signin for first cloud user#17834
JanKaczmarkiewicz merged 41 commits intomasterfrom
jan/cloud-passwordless-default-preference

Conversation

@JanKaczmarkiewicz
Copy link
Copy Markdown
Contributor

@JanKaczmarkiewicz JanKaczmarkiewicz commented Oct 26, 2022

What

Automatically set the passwordless cluster authentication preference for cloud users.

When the first user chooses to sign in with passwordless, cluster_auth_preference.connector_name will be set to "passwordless". This will instruct login and sign-in flow to use passwordless as the default method (the user can still choose another available method).

Testing guide:

  1. checkout to the branch (make sure teleport.e is cloned)
  2. Create a new tenant in the staging sales-center
  3. build a dev version of teleport-ent and use it in the staging tenant (see https://github.com/gravitational/teleport.e/blob/master/dev-deploy.md for exact steps)
    If you run into a missing DEB_PATH error you can temporarily restore build.assets/charts/Dockerfile old code before https://github.com/gravitational/teleport/pull/17597/files#diff-3b89b68ec0c7abb15cbf57af77d2f3cced352cbe65159772d3b680ef76f9915e
  4. Go to the invite link created in step 2. and use the passwordless form to authenticate
  5. In Web UI create a second user.
  6. After entering an invite link the second user should see the passwordless form initially instead of the password + TOTP form

Implements RFD: https://github.com/gravitational/cloud/pull/2405
Fixes: https://github.com/gravitational/cloud/issues/2069

@JanKaczmarkiewicz JanKaczmarkiewicz marked this pull request as ready for review November 7, 2022 15:55
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver_test.go Outdated
Comment thread lib/web/apiserver.go Outdated
@jimbishopp jimbishopp requested a review from nklaassen November 7, 2022 19:51
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/auth/helpers.go Outdated
Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/apiserver_test.go Outdated
Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the updating the PR description too.

Holding out a bit more for:

  1. Tests for clusters with non-"local" connectors and
  2. A resolution to the "NewTestAuthServer" thread

Comment thread lib/web/apiserver_test.go Outdated
@JanKaczmarkiewicz JanKaczmarkiewicz enabled auto-merge (squash) November 14, 2022 14:00
@JanKaczmarkiewicz JanKaczmarkiewicz enabled auto-merge (squash) November 14, 2022 14:45
@JanKaczmarkiewicz JanKaczmarkiewicz enabled auto-merge (squash) November 14, 2022 14:50
@JanKaczmarkiewicz JanKaczmarkiewicz merged commit 2a9167b into master Nov 14, 2022
JanKaczmarkiewicz added a commit that referenced this pull request Nov 14, 2022
* feat: set cluster connector name on first user signin

* perf: move is passwordless enabled before get users

* fix: move logic to correct handler

* test: test setting default passwordless connector name for cloud (positive case)

* test: add negative test cases

* refactor: flatten nested code

* fix: remove return in case auth preference was not set

* fix: run logic only in cloud

* docs: typo

* fix: run when passwordless

* test: move trySettingConnectorNameToPasswordless as separete method

* docs: add safety comments

* test: rephrase safety comment

* refactor: remove space between call and error check

* fix: remove unnessesery error log from trySettingConnectorNameToPasswordless

* refactor: pass context, rename session context

* docs: add description to trySettingConnectorNameToPasswordless

* test: add descriptive assertion text

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* refactor: simplify error handilng

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* fix: check for non default setting

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: correct RPID

* refactor: simplify user creation

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* docs: fix typo

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: reorder name filed in the test cases struct

* test: better names for test cases

* refactor: generate token instead of hardcoding arbitrary value

* test: remove user agent header

* fix: check for empty password

* docs: add passwordless check description

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: add "first cloud sign-in does not change custom connector"

* test: use correct number of users in "first cloud sign-in does not change custom connector"

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: remove is cloud check from test helpers, instead use config

* refactor: store is_passwordless_registration in variable

* test: first cloud sign-in with password does not change connector

* refactor: change isPasswordlessRegistration name

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@zmb3 zmb3 deleted the jan/cloud-passwordless-default-preference branch November 14, 2022 17:40
JanKaczmarkiewicz added a commit that referenced this pull request Nov 16, 2022
…18445)

* feat: set cluster connector name on first user signin

* perf: move is passwordless enabled before get users

* fix: move logic to correct handler

* test: test setting default passwordless connector name for cloud (positive case)

* test: add negative test cases

* refactor: flatten nested code

* fix: remove return in case auth preference was not set

* fix: run logic only in cloud

* docs: typo

* fix: run when passwordless

* test: move trySettingConnectorNameToPasswordless as separete method

* docs: add safety comments

* test: rephrase safety comment

* refactor: remove space between call and error check

* fix: remove unnessesery error log from trySettingConnectorNameToPasswordless

* refactor: pass context, rename session context

* docs: add description to trySettingConnectorNameToPasswordless

* test: add descriptive assertion text

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* refactor: simplify error handilng

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* fix: check for non default setting

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: correct RPID

* refactor: simplify user creation

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* docs: fix typo

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: reorder name filed in the test cases struct

* test: better names for test cases

* refactor: generate token instead of hardcoding arbitrary value

* test: remove user agent header

* fix: check for empty password

* docs: add passwordless check description

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: add "first cloud sign-in does not change custom connector"

* test: use correct number of users in "first cloud sign-in does not change custom connector"

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* test: remove is cloud check from test helpers, instead use config

* refactor: store is_passwordless_registration in variable

* test: first cloud sign-in with password does not change connector

* refactor: change isPasswordlessRegistration name

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants