Skip to content

Commit

Permalink
fix(auth): allow authorization for Github and OIDC behind the reverse
Browse files Browse the repository at this point in the history
proxy

Signed-off-by: Roman Dmytrenko <[email protected]>
  • Loading branch information
erka committed May 29, 2024
1 parent 985491c commit 24ded33
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 42 deletions.
1 change: 1 addition & 0 deletions examples/proxy/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ server {
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Prefix /flipt;
proxy_buffering on;

# rewrite
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/authn.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func authenticationHTTPMount(
}

if cfg.SessionEnabled() {
muxOpts = append(muxOpts, runtime.WithMetadata(method.ForwardCookies))
muxOpts = append(muxOpts, runtime.WithMetadata(method.ForwardCookies), runtime.WithMetadata(method.ForwardPrefix))

Check warning on line 336 in internal/cmd/authn.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/authn.go#L336

Added line #L336 was not covered by tests

methodMiddleware := method.NewHTTPMiddleware(cfg.Session)
muxOpts = append(muxOpts, runtime.WithForwardResponseOption(methodMiddleware.ForwardResponseOption))
Expand Down
8 changes: 1 addition & 7 deletions internal/config/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,7 @@ func (a AuthenticationMethodOIDCConfig) info() AuthenticationMethodInfo {
// this ensures we expose the authorize and callback URL endpoint
// to the UI via the /auth/v1/method endpoint
for provider := range a.Providers {
providers[provider] = map[string]any{
"authorize_url": fmt.Sprintf("/auth/v1/method/oidc/%s/authorize", provider),
"callback_url": fmt.Sprintf("/auth/v1/method/oidc/%s/callback", provider),
}
providers[provider] = map[string]any{}
}

metadata["providers"] = providers
Expand Down Expand Up @@ -552,9 +549,6 @@ func (a AuthenticationMethodGithubConfig) info() AuthenticationMethodInfo {

metadata := make(map[string]any)

metadata["authorize_url"] = "/auth/v1/method/github/authorize"
metadata["callback_url"] = "/auth/v1/method/github/callback"

info.Metadata, _ = structpb.NewStruct(metadata)

return info
Expand Down
33 changes: 25 additions & 8 deletions internal/server/authn/method/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import (
"google.golang.org/protobuf/proto"
)

var (
githubPrefix = "/auth/v1/method/github"
oidcPrefix = "/auth/v1/method/oidc/"
stateCookieKey = "flipt_client_state"
tokenCookieKey = "flipt_client_token"
const (
githubPrefix = "/auth/v1/method/github"
oidcPrefix = "/auth/v1/method/oidc/"
stateCookieKey = "flipt_client_state"
tokenCookieKey = "flipt_client_token"
forwardedPrefixKey = "x-forwarded-prefix"
xForwardedPrefix = "X-Forwarded-Prefix"
)

// Middleware contains various extensions for appropriate integration of the OIDC services
Expand Down Expand Up @@ -53,6 +55,17 @@ func ForwardCookies(ctx context.Context, req *http.Request) metadata.MD {
return md
}

// ForwardPrefix extracts the "X-Forwarded-Prefix" header from an HTTP request
// and forwards them as grpc metadata entries.
func ForwardPrefix(ctx context.Context, req *http.Request) metadata.MD {
md := metadata.MD{}
values := req.Header.Values(xForwardedPrefix)
if len(values) > 0 {
md[forwardedPrefixKey] = values
}
return md
}

// ForwardResponseOption is a grpc gateway forward response option function implementation.
// The purpose of which is to intercept outgoing Callback operation responses.
// When intercepted the resulting clientToken is stripped from the response payload and instead
Expand All @@ -77,8 +90,11 @@ func (m Middleware) ForwardResponseOption(ctx context.Context, w http.ResponseWr

// clear out token now that it is set via cookie
r.ClientToken = ""

w.Header().Set("Location", "/")
location := "/"
if md, ok := metadata.FromOutgoingContext(ctx); ok {
location = path.Join(md.Get(forwardedPrefixKey)...) + "/"

Check warning on line 95 in internal/server/authn/method/http.go

View check run for this annotation

Codecov / codecov/patch

internal/server/authn/method/http.go#L93-L95

Added lines #L93 - L95 were not covered by tests
}
w.Header().Set("Location", location)

Check warning on line 97 in internal/server/authn/method/http.go

View check run for this annotation

Codecov / codecov/patch

internal/server/authn/method/http.go#L97

Added line #L97 was not covered by tests
w.WriteHeader(http.StatusFound)
}

Expand All @@ -101,6 +117,7 @@ func (m Middleware) Handler(next http.Handler) http.Handler {
}

if method == "authorize" {
prefix = path.Join(r.Header.Get(xForwardedPrefix), prefix)

Check warning on line 120 in internal/server/authn/method/http.go

View check run for this annotation

Codecov / codecov/patch

internal/server/authn/method/http.go#L120

Added line #L120 was not covered by tests
query := r.URL.Query()
// create a random security token and bind it to
// the state parameter while preserving any provided
Expand Down Expand Up @@ -130,7 +147,7 @@ func (m Middleware) Handler(next http.Handler) http.Handler {
Name: stateCookieKey,
Value: encoded,
// bind state cookie to provider callback
Path: prefix + "callback",
Path: prefix + "/callback",

Check warning on line 150 in internal/server/authn/method/http.go

View check run for this annotation

Codecov / codecov/patch

internal/server/authn/method/http.go#L150

Added line #L150 was not covered by tests
Expires: time.Now().Add(m.config.StateLifetime),
Secure: m.config.Secure,
HttpOnly: true,
Expand Down
30 changes: 30 additions & 0 deletions internal/server/authn/method/http_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package method

import (
"context"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
)

func TestForwardPrefix(t *testing.T) {
tests := []struct {
name string
headers map[string]string
expected []string
}{
{"forward", map[string]string{"X-Forwarded-Prefix": "/my-prefix"}, []string{"/my-prefix"}},
{"none", map[string]string{}, nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := httptest.NewRequest("GET", "http://example.com/foo", nil)
for k, v := range tt.headers {
req.Header.Add(k, v)
}
md := ForwardPrefix(context.Background(), req)
assert.Equal(t, tt.expected, md.Get(forwardedPrefixKey))
})
}
}
1 change: 1 addition & 0 deletions internal/server/authn/method/oidc/testing/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func StartHTTPServer(
mux = gateway.NewGatewayServeMux(
logger,
runtime.WithMetadata(method.ForwardCookies),
runtime.WithMetadata(method.ForwardPrefix),
runtime.WithForwardResponseOption(oidcmiddleware.ForwardResponseOption),
)
)
Expand Down
20 changes: 7 additions & 13 deletions ui/src/app/auth/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import logoFlag from '~/assets/logo-flag.png';
import Loading from '~/components/Loading';
import { NotificationProvider } from '~/components/NotificationProvider';
import ErrorNotification from '~/components/notifications/ErrorNotification';
import { authURL } from '~/data/api';
import { useError } from '~/data/hooks/error';
import { useSession } from '~/data/hooks/session';
import { IAuthMethod } from '~/types/Auth';
Expand All @@ -26,7 +27,6 @@ interface ILoginProvider {
interface IAuthDisplay {
name: string;
authorize_url: string;
callback_url: string;
icon: IconDefinition;
}

Expand Down Expand Up @@ -93,23 +93,17 @@ function InnerLoginButtons() {
if (m.method === 'METHOD_GITHUB') {
return {
name: 'Github',
authorize_url: m.metadata.authorize_url,
callback_url: m.metadata.callback_url,
authorize_url: `${authURL}/method/github/authorize`,
icon: faGithub
};
}
if (m.method === 'METHOD_OIDC') {
return Object.entries(m.metadata.providers).map(([k, value]) => {
k = k.toLowerCase();
const v = value as {
authorize_url: string;
callback_url: string;
};
return Object.keys(m.metadata.providers).map((k) => {
const kl = k.toLowerCase();
return {
name: knownProviders[k]?.displayName || upperFirst(k), // if we dont know the provider, just capitalize the first letter
authorize_url: v.authorize_url,
callback_url: v.callback_url,
icon: knownProviders[k]?.icon || faOpenid // if we dont know the provider icon, use the openid icon
name: knownProviders[kl]?.displayName || upperFirst(kl), // if we dont know the provider, just capitalize the first letter
authorize_url: `${authURL}/method/oidc/${k}/authorize`,
icon: knownProviders[kl]?.icon || faOpenid // if we dont know the provider icon, use the openid icon
};
});
}
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/header/UserProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default function UserProfile(props: UserProfileProps) {
let name: string | undefined;
let login: string | undefined;
let imgURL: string | undefined;
let logoutURL = '/';
let logoutURL = '';

if (session) {
const authMethods = ['github', 'oidc', 'jwt'];
Expand Down
5 changes: 1 addition & 4 deletions ui/src/types/auth/Github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ import { IAuth, IAuthMethod } from '~/types/Auth';

export interface IAuthMethodGithub extends IAuthMethod {
method: 'METHOD_GITHUB';
metadata: {
authorize_url: string;
callback_url: string;
};
metadata: {};
}

export interface IAuthMethodGithubMetadata {
Expand Down
5 changes: 1 addition & 4 deletions ui/src/types/auth/JWT.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ import { IAuth, IAuthMethod } from '~/types/Auth';

export interface IAuthMethodJWT extends IAuthMethod {
method: 'METHOD_JWT';
metadata: {
authorize_url: string;
callback_url: string;
};
metadata: {};
}

export interface IAuthMethodJWTMetadata {
Expand Down
5 changes: 1 addition & 4 deletions ui/src/types/auth/OIDC.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { IAuth, IAuthMethod } from '~/types/Auth';

interface AuthMethodOIDCMetadataProvider {
authorize_url: string;
callback_url: string;
}
interface AuthMethodOIDCMetadataProvider {}

export interface IAuthMethodOIDC extends IAuthMethod {
method: 'METHOD_OIDC';
Expand Down

0 comments on commit 24ded33

Please sign in to comment.