Skip to content

feat: add support for optional external auth providers #12021

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

Merged
merged 35 commits into from
Feb 21, 2024

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Feb 5, 2024

Screenshot 2024-02-14 at 4 34 01 PM

(screenshot includes frontend changes that will be landing in a separate pull request, because this one is already big enough 😄)

Part of #10109

  • Migrate external_auth_providers column in the database, in order to be able to contain metadata about providers, such as settings/attributes defined in Terraform.
  • Add support for external_auth_provider.optional in the Terraform provisioner and provisionerd
  • Add optional to TemplateVersionExternalAuth
    • and fill it with new database goodness when querying /templateversions/{templateversion}/external-auth

TODO

  • who uses proto.CurrentVersion? there are some tests failing because of this, and I'm not really sure what to do about it
    • also it doesn't seem like there's any machinery for checking the terraform provider version? kind of unfortunate that you could just start using 0.16 with an old version of coder and optional would just silently disappear

@aslilac aslilac force-pushed the optional-external-auth branch from b83538b to 179adae Compare February 9, 2024 22:00
@aslilac aslilac requested a review from spikecurtis February 14, 2024 23:49
@aslilac aslilac requested a review from johnstcn February 15, 2024 18:04
@aslilac aslilac requested a review from spikecurtis February 15, 2024 19:16
@aslilac aslilac marked this pull request as ready for review February 15, 2024 20:21
@kylecarbs
Copy link
Member

Regarding this UX, should we revert to the old UI? The "Connect" button with "Optional" and then also presumably a "Skip" feels like a lot.

@aslilac
Copy link
Member Author

aslilac commented Feb 15, 2024

Regarding this UX, should we revert to the old UI? The "Connect" button with "Optional" and then also presumably a "Skip" feels like a lot.

it sounds like everyone is in favor of a revert. the screenshot is of some very very placeholder coder for testing that isn't even in this pr. the frontend stuff will be a separate pr.

Copy link
Member

Choose a reason for hiding this comment

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

heads-up: just before you merge, double-check the latest migration number on main

@aslilac aslilac requested a review from johnstcn February 20, 2024 20:47
@aslilac aslilac merged commit 475c365 into main Feb 21, 2024
@aslilac aslilac deleted the optional-external-auth branch February 21, 2024 18:18
@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2024
@aslilac aslilac linked an issue Feb 21, 2024 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make external auth providers optional
4 participants