NotificationsYou must be signed in to change notification settings
Fork927
Star10.1k

feat: add`owner_oidc_access_token` to`coder_workspace` data source#6042

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

kylecarbs merged 2 commits intomainfromprovisionoidc

Mar 17, 2023

Merged

feat: add`owner_oidc_access_token` to`coder_workspace` data source#6042

kylecarbs merged 2 commits intomainfromprovisionoidc

Mar 17, 2023

Conversation

Copy link

Member

kylecarbs commentedFeb 5, 2023

See the discussion in Discord here:
https://discord.com/channels/747933592273027093/1071182088490987542/1071182088490987542

Related provider PR:coder/terraform-provider-coder#91

feat: addowner_oidc_access_token tocoder_workspace data source

760c4a8

See the discussion in Discord here:https://discord.com/channels/747933592273027093/1071182088490987542/1071182088490987542Related provider PR:coder/terraform-provider-coder#91

kylecarbs requested a review fromdeansheather

February 5, 2023 21:49

kylecarbs self-assigned this

Feb 5, 2023

deansheather reviewed

Feb 6, 2023

View reviewed changes

Copy link

Member

deansheather left a comment

Choose a reason for hiding this comment

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

This needs to be configurable via a flag. I think it should be disabled by default personally, but at the very least it needs to be the other way around IMO.

provisionersdk/proto/provisioner.proto Outdated

		@@ -201,6 +201,7 @@ message Provision {
		string workspace_id = 5;
		string workspace_owner_id = 6;
		string workspace_owner_email = 7;
		string workspace_owner_oidc_access_token = 8;

Copy link

Member

deansheatherFeb 6, 2023

Choose a reason for hiding this comment

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

tabs vs spaces

Copy link

MemberAuthor

kylecarbs commentedFeb 6, 2023

@deansheather why does this need to be configurable via a flag?

Copy link

Member

deansheather commentedFeb 6, 2023

To prevent template admins from reading OIDC secrets without deployment approval. It seems dangerous to allow anyone with write access to templates to push an update that starts reading OIDC tokens and doing nefarious things with them.

We have a similar feature in v1 and it's disabled by default for this reason

Copy link

MemberAuthor

kylecarbs commentedFeb 6, 2023

Hmm, fair. I suppose this would permit any template author to gain access to users inside of Coder, so it could be real bad. Some additional thought is needed on how we do it... 🤔

Copy link

Member

bpmct commentedFeb 8, 2023•
edited
Loading

Can we make it disabled by default?

Copy link

github-actionsbot commentedFeb 16, 2023

This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity.