Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Commit

Permalink
fix: remove unused groups OIDC scope (#558)
Browse files Browse the repository at this point in the history
Fixes #557
  • Loading branch information
leg100 committed Aug 5, 2023
1 parent 031d673 commit 3dd465a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 16 deletions.
8 changes: 4 additions & 4 deletions cmd/otfd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ func parseFlags(ctx context.Context, args []string, out io.Writer) error {
cmd.Flags().StringVar(&cfg.Gitlab.OAuthConfig.ClientID, "gitlab-client-id", "", "gitlab client ID")
cmd.Flags().StringVar(&cfg.Gitlab.OAuthConfig.ClientSecret, "gitlab-client-secret", "", "gitlab client secret")

cmd.Flags().StringVar(&cfg.OIDC.Name, "oidc-name", cfg.OIDC.Name, "user friendly oidc name")
cmd.Flags().StringVar(&cfg.OIDC.IssuerURL, "oidc-issuer-url", cfg.OIDC.IssuerURL, "oidc issuer url")
cmd.Flags().StringVar(&cfg.OIDC.ClientID, "oidc-client-id", "", "oidc client ID")
cmd.Flags().StringVar(&cfg.OIDC.ClientSecret, "oidc-client-secret", "", "oidc client secret")
cmd.Flags().StringVar(&cfg.OIDC.Name, "oidc-name", cfg.OIDC.Name, "User friendly OIDC name")
cmd.Flags().StringVar(&cfg.OIDC.IssuerURL, "oidc-issuer-url", cfg.OIDC.IssuerURL, "OIDC issuer URL")
cmd.Flags().StringVar(&cfg.OIDC.ClientID, "oidc-client-id", "", "OIDC client ID")
cmd.Flags().StringVar(&cfg.OIDC.ClientSecret, "oidc-client-secret", "", "OIDC client secret")

cmd.Flags().BoolVar(&cfg.RestrictOrganizationCreation, "restrict-org-creation", false, "Restrict organization creation capability to site admin role")

Expand Down
4 changes: 2 additions & 2 deletions docs/auth/providers/oidc.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ You can configure OTF to sign users in using [OpenID-Connect](https://openid.net
Create an Application for OTF in your preferred IdP.

* Set the name to something appropriate, e.g. `otf`
* Select required scopes `openid`, `profile`, and `groups`
* Select required scopes `openid`, `profile`
* Add the following `redirect uri` to the application:

`https://<otfd_install_hostname>/oauth/<oidc_name>/callback`
Expand All @@ -15,7 +15,7 @@ Once you've created the application note the client id and secret.
Set the following flags when running otfd:

* `--oidc-name=<oidc_name>` which is the user-friendly name of the idp. This can be something like `azure-sso`, or `google`.
* `--oidc-issuer-url=<issuer-url>` which is the URL of the idp's oidc configuration. This varies depending on the identity provider used.
* `--oidc-issuer-url=<issuer-url>` which is the URL of the idp's OIDC configuration. This varies depending on the identity provider used.
* `--oidc-client-id=<client-id>` which is the `client-id` generated by the idp when we created the application.
* `--oidc-client-secret=<client-secret>` which is the `client-secret` generated by the idp when we created the application.

Expand Down
27 changes: 19 additions & 8 deletions internal/authenticator/oidc_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package authenticator

import (
"context"
"errors"
"fmt"
"net/http"

"github.com/coreos/go-oidc/v3/oidc"
Expand All @@ -13,10 +15,18 @@ import (
"golang.org/x/oauth2"
)

var _ authenticator = &oidcAuthenticator{}
var (
_ authenticator = &oidcAuthenticator{}

// "openid" is a required scope for OpenID Connect flows, and profile
// gives OTF access to the user's username.
DefaultScopes = []string{oidc.ScopeOpenID, "profile"}

ErrMissingOIDCIssuerURL = errors.New("missing oidc-issuer-url")
)

type (
// oidcAuthenticator is an authenticator that uses oidc.
// oidcAuthenticator is an authenticator that uses OIDC.
oidcAuthenticator struct {
tokens.TokensService // for creating session

Expand All @@ -33,14 +43,18 @@ type (
cloud.OIDCConfig
}

// oidcClaims depicts the claims returned from the oidc id-token.
// oidcClaims depicts the claims returned from the OIDC id-token.
oidcClaims struct {
Name string `json:"name"`
Groups []string `json:"groups"`
}
)

func newOIDCAuthenticator(ctx context.Context, opts oidcAuthenticatorOptions) (*oidcAuthenticator, error) {
if opts.IssuerURL == "" {
return nil, ErrMissingOIDCIssuerURL
}

cloudConfig := cloud.Config{
Name: opts.Name,
SkipTLSVerification: opts.SkipTLSVerification,
Expand All @@ -51,7 +65,7 @@ func newOIDCAuthenticator(ctx context.Context, opts oidcAuthenticatorOptions) (*
ctx = oidc.ClientContext(ctx, cloudConfig.HTTPClient())
provider, err := oidc.NewProvider(ctx, opts.IssuerURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("constructing OIDC provider: %w", err)
}

return &oidcAuthenticator{
Expand All @@ -65,10 +79,7 @@ func newOIDCAuthenticator(ctx context.Context, opts oidcAuthenticatorOptions) (*
ClientID: opts.ClientID,
ClientSecret: opts.ClientSecret,
Endpoint: provider.Endpoint(),

// "openid" is a required scope for OpenID Connect flows.
// groups is used for managing permissions.
Scopes: append(opts.Scopes, oidc.ScopeOpenID, "groups", "profile"),
Scopes: DefaultScopes,
},
cloudConfig: cloudConfig,
},
Expand Down
7 changes: 7 additions & 0 deletions internal/authenticator/oidc_authenticator_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package authenticator

import (
"context"
"crypto/rand"
"crypto/rsa"
"net/http"
Expand All @@ -11,6 +12,12 @@ import (
"github.com/stretchr/testify/require"
)

func TestNewOIDCAuthenticator(t *testing.T) {
ctx := context.Background()
_, err := newOIDCAuthenticator(ctx, oidcAuthenticatorOptions{})
assert.Equal(t, ErrMissingOIDCIssuerURL, err)
}

func TestOIDCAuthenticator_ResponseHandler(t *testing.T) {
priv, err := rsa.GenerateKey(rand.Reader, 512)
require.NoError(t, err)
Expand Down
2 changes: 0 additions & 2 deletions internal/cloud/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ type OIDCConfig struct {
ClientID string
// ClientSecret is the client secret for the oidc provider.
ClientSecret string
// Scopes is a list of optional scopes to pass to the oidc provider.
Scopes []string
// Skip TLS Verification when communicating with issuer
SkipTLSVerification bool
}

0 comments on commit 3dd465a

Please sign in to comment.