Skip to content
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

enable oauth clients on demand #57

Merged
merged 3 commits into from
Jan 5, 2021
Merged

Conversation

nivemaham
Copy link
Member

enable oauth clients

@nivemaham nivemaham changed the title enable oauth clients enable oauth clients on demand Oct 29, 2020
@blootsvoets
Copy link
Member

As a question: do we want to be backwards compatible? If not, I'd propose to have a unified view, like

oauthClients:
  pRMT:
    enable: false
    resource_ids: res_gateway,res_ManagementPortal
    client_secret: ""
    scope: MEASUREMENT.CREATE,SUBJECT.UPDATE,SUBJECT.READ,PROJECT.READ,SOURCETYPE.READ,SOURCE.READ,SOURCETYPE.READ,SOURCEDATA.READ,USER.READ,ROLE.READ
    authorized_grant_types: refresh_token,authorization_code
    redirect_uri: ""
    authorities: ""
    access_token_validity: 43200
    refresh_token_validity: 7948800
    additional_information: '{"dynamic_registration": true}'
    autoapprove: ""

etc... for other clients. Then the template would be something like

client_id;resource_ids;client_secret;scope;authorized_grant_types;redirect_uri;authorities;access_token_validity;refresh_token_validity;additional_information;autoapprove
{{- range $clientId, $client := .Values.oauthClients -}}
{{- if $client.enable }}
{{ $clientId }};{{ $client.resource_ids }};{{ $client.client_secret }};{{ $client.scope }};{{ $client.authorized_grant_types }};{{ $client.redirect_uri | default "" }};{{ $client.authorities | default "" }};{{ $client.access_token_validity }};{{ $client. refresh_token_validity | default "0" }};{{ $client.additional_information | default "" }};{{ $client.autoapprove | default "" }}
{{- end -}}
{{- end -}}

Then in values, you can easily enable existing clients with values

oauthClients:
  pRMT:
    enable: true
    secret: mySecret

and add your own clients as well.

@nivemaham
Copy link
Member Author

I think we can do the change we suggested as well. It would be a bigger change though. I find current setup easier to maintain. Plus do we want to put the oauth client details of a specific component inside the chart ?

@keyvaann
Copy link
Collaborator

The suggestions from Joris seem good to me and I think we can break backward compatibility here as we don't use this project in many installations yet.

@nivemaham
Copy link
Member Author

nivemaham commented Nov 3, 2020

I am getting the error message below when I run helmfile -f filename template

  Error: template: management-portal/templates/deployment.yaml:19:31: executing "management-portal/templates/deployment.yaml" at <include (print $.Template.BasePath "/configmap.yaml") .>: error calling include: template: management-portal/templates/configmap.yaml:14:18: executing "management-portal/templates/configmap.yaml" at <$client.enable>: can't evaluate field enable in type interface {}
  

Any idea how to iterate through this custom model?
I think the below model would work instead, then enabling clients based on name would be tricky.

oauthClients:
  - enable: false
    resource_ids: res_gateway,res_ManagementPortal
    client_secret: ""
    scope: MEASUREMENT.CREATE,SUBJECT.UPDATE,SUBJECT.READ,PROJECT.READ,SOURCETYPE.READ,SOURCE.READ,SOURCETYPE.READ,SOURCEDATA.READ,USER.READ,ROLE.READ
    authorized_grant_types: refresh_token,authorization_code
    redirect_uri: ""
    authorities: ""
    access_token_validity: 43200
    refresh_token_validity: 7948800
    additional_information: '{"dynamic_registration": true}'
    autoapprove: ""

@K1Hyve @blootsvoets Any thoughts?

@keyvaann
Copy link
Collaborator

keyvaann commented Nov 3, 2020

@nivemaham I think you should quote the enable value and put "false" instead

@blootsvoets
Copy link
Member

blootsvoets commented Nov 3, 2020

Maybe because you're using an array instead of an object? It's complaining that it can't find your object ({}). {{- range $clientId, $client := .Values.oauthClients -}} only works if the oauthClients is a map. enable should be a boolean rather than a string (as you already wrote), because it is evaluated as such.

@blootsvoets
Copy link
Member

@K1Hyve @nivemaham refactored the PR to have fully configurable OAuth 2.0 clients. Coul you please review?

@blootsvoets blootsvoets merged commit 23422ee into dev Jan 5, 2021
@blootsvoets blootsvoets deleted the enable-oauth-clients-ondemand branch January 5, 2021 13:06
blootsvoets added a commit that referenced this pull request Dec 21, 2022
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.

3 participants