Route connect OAuth through unified sign-in
Drop the portal-login redirect, accept continue URLs for magic links, brand emails from the OAuth client when present, and update e2e coverage. Signed-off-by: Bryan Frimin <bryan@probo.com>
This commit is contained in:
@@ -38,7 +38,7 @@ func (r *mutationResolver) SendMagicLink(ctx context.Context, input types.SendMa
|
||||
req := &iam.SendMagicLinkRequest{
|
||||
Email: input.Email,
|
||||
CompliancePageID: &trustCenter.ID,
|
||||
OrganizationID: trustCenter.OrganizationID,
|
||||
OrganizationID: &trustCenter.OrganizationID,
|
||||
URLPath: "verify-magic-link",
|
||||
Continue: input.Continue,
|
||||
}
|
||||
|
||||
@@ -25,6 +25,7 @@ import (
|
||||
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/baseurl"
|
||||
trust "go.probo.inc/probo/pkg/complianceportal/visitor"
|
||||
"go.probo.inc/probo/pkg/filemanager"
|
||||
"go.probo.inc/probo/pkg/iam"
|
||||
"go.probo.inc/probo/pkg/securecookie"
|
||||
@@ -36,13 +37,22 @@ import (
|
||||
"go.probo.inc/probo/pkg/server/gqlutils/directives/session"
|
||||
)
|
||||
|
||||
func NewGraphQLHandler(svc *iam.Service, logger *log.Logger, fileManagerSvc *filemanager.Service, baseURL *baseurl.BaseURL, cookieConfig securecookie.Config, limits gqlutils.Limits) http.Handler {
|
||||
func NewGraphQLHandler(
|
||||
svc *iam.Service,
|
||||
trustSvc *trust.Service,
|
||||
logger *log.Logger,
|
||||
fileManagerSvc *filemanager.Service,
|
||||
baseURL *baseurl.BaseURL,
|
||||
cookieConfig securecookie.Config,
|
||||
limits gqlutils.Limits,
|
||||
) http.Handler {
|
||||
config := schema.Config{
|
||||
Resolvers: &Resolver{
|
||||
authorize: authz.NewAuthorizeFunc(svc, logger),
|
||||
batchAuthorize: authz.NewBatchAuthorizeFunc(svc, logger),
|
||||
logger: logger,
|
||||
iam: svc,
|
||||
trust: trustSvc,
|
||||
scopeRegistry: svc.OAuth2ScopeRegistry,
|
||||
fileManager: fileManagerSvc,
|
||||
baseURL: baseURL,
|
||||
|
||||
109
pkg/server/api/connect/v1/magic_link_handler_test.go
Normal file
109
pkg/server/api/connect/v1/magic_link_handler_test.go
Normal file
@@ -0,0 +1,109 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
|
||||
package connect_v1_test
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/baseurl"
|
||||
"go.probo.inc/probo/pkg/securecookie"
|
||||
connect_v1 "go.probo.inc/probo/pkg/server/api/connect/v1"
|
||||
)
|
||||
|
||||
func newTestMagicLinkHandler(t *testing.T) *connect_v1.MagicLinkHandler {
|
||||
t.Helper()
|
||||
|
||||
return connect_v1.NewMagicLinkHandler(
|
||||
nil,
|
||||
baseurl.MustParse("https://auth.example.com"),
|
||||
securecookie.Config{Secret: "01234567890123456789012345678901"},
|
||||
log.NewLogger(log.WithOutput(io.Discard)),
|
||||
nil,
|
||||
)
|
||||
}
|
||||
|
||||
func postMagicLinkForm(t *testing.T, handler http.HandlerFunc, values url.Values) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost,
|
||||
"/api/connect/v1/magic-link/send",
|
||||
strings.NewReader(values.Encode()),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler(rec, req)
|
||||
|
||||
return rec
|
||||
}
|
||||
|
||||
func TestMagicLinkHandler_SendHandler_Validation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
handler := newTestMagicLinkHandler(t)
|
||||
|
||||
t.Run("rejects missing continue", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := postMagicLinkForm(
|
||||
t,
|
||||
handler.SendHandler,
|
||||
url.Values{
|
||||
"email": {"user@example.com"},
|
||||
},
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
})
|
||||
|
||||
t.Run("rejects invalid email", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := postMagicLinkForm(
|
||||
t,
|
||||
handler.SendHandler,
|
||||
url.Values{
|
||||
"email": {"not-an-email"},
|
||||
"continue": {"/"},
|
||||
},
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
})
|
||||
|
||||
t.Run("rejects unsafe continue URL", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := postMagicLinkForm(
|
||||
t,
|
||||
handler.SendHandler,
|
||||
url.Values{
|
||||
"email": {"user@example.com"},
|
||||
"continue": {"//evil.example.com"},
|
||||
},
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
})
|
||||
|
||||
}
|
||||
36
pkg/server/api/connect/v1/oauth2_client_id.go
Normal file
36
pkg/server/api/connect/v1/oauth2_client_id.go
Normal file
@@ -0,0 +1,36 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
|
||||
package connect_v1
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
)
|
||||
|
||||
const oauth2AuthorizeFullPath = "/api/connect/v1" + oauth2AuthorizePath
|
||||
|
||||
func oauth2ClientIDFromContinueURL(continueURL string) string {
|
||||
parsed, err := url.Parse(continueURL)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
// Only read client_id from the real authorize endpoint. The continue URL
|
||||
// itself is already validated by saferedirect before this runs.
|
||||
if parsed.Path != oauth2AuthorizeFullPath {
|
||||
return ""
|
||||
}
|
||||
|
||||
return parsed.Query().Get("client_id")
|
||||
}
|
||||
65
pkg/server/api/connect/v1/oauth2_client_id_test.go
Normal file
65
pkg/server/api/connect/v1/oauth2_client_id_test.go
Normal file
@@ -0,0 +1,65 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
|
||||
package connect_v1
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestOauth2ClientIDFromContinueURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
continueURL string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "extracts client_id from authorize URL",
|
||||
continueURL: "/api/connect/v1/oauth2/authorize?client_id=https%3A%2F%2Ftrust.example.com%2F.well-known%2Foauth-client-metadata&response_type=code",
|
||||
want: "https://trust.example.com/.well-known/oauth-client-metadata",
|
||||
},
|
||||
{
|
||||
name: "returns empty for non-authorize URL",
|
||||
continueURL: "/overview",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "returns empty for path suffix lookalike",
|
||||
continueURL: "/evil/oauth2/authorize?client_id=phishing",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "returns empty when client_id is missing",
|
||||
continueURL: "/api/connect/v1/oauth2/authorize?response_type=code",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "extracts client_id from absolute authorize URL",
|
||||
continueURL: "https://auth.example.com/api/connect/v1/oauth2/authorize?client_id=gid%3A%2F%2Fprobo%2Foauth2_client%2Fabc",
|
||||
want: "gid://probo/oauth2_client/abc",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(t, tt.want, oauth2ClientIDFromContinueURL(tt.continueURL))
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -32,7 +32,6 @@ import (
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/baseurl"
|
||||
"go.probo.inc/probo/pkg/bearertoken"
|
||||
trust "go.probo.inc/probo/pkg/complianceportal/visitor"
|
||||
"go.probo.inc/probo/pkg/coredata"
|
||||
"go.probo.inc/probo/pkg/iam"
|
||||
"go.probo.inc/probo/pkg/iam/oauth2"
|
||||
@@ -43,29 +42,23 @@ import (
|
||||
)
|
||||
|
||||
type OAuth2Handler struct {
|
||||
iam *iam.Service
|
||||
trust *trust.Service
|
||||
sessionCookie *authn.Cookie
|
||||
baseURL *baseurl.BaseURL
|
||||
portalLoginPath string
|
||||
logger *log.Logger
|
||||
iam *iam.Service
|
||||
sessionCookie *authn.Cookie
|
||||
baseURL *baseurl.BaseURL
|
||||
logger *log.Logger
|
||||
}
|
||||
|
||||
func NewOAuth2Handler(
|
||||
svc *iam.Service,
|
||||
trustSvc *trust.Service,
|
||||
cookieConfig securecookie.Config,
|
||||
baseURL *baseurl.BaseURL,
|
||||
portalLoginPath string,
|
||||
logger *log.Logger,
|
||||
) *OAuth2Handler {
|
||||
return &OAuth2Handler{
|
||||
iam: svc,
|
||||
trust: trustSvc,
|
||||
sessionCookie: authn.NewCookie(&cookieConfig),
|
||||
baseURL: baseURL,
|
||||
portalLoginPath: portalLoginPath,
|
||||
logger: logger.Named("oauth2"),
|
||||
iam: svc,
|
||||
sessionCookie: authn.NewCookie(&cookieConfig),
|
||||
baseURL: baseURL,
|
||||
logger: logger.Named("oauth2"),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -150,16 +143,6 @@ func (h *OAuth2Handler) AuthorizeHandler(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
|
||||
clientID := r.URL.Query().Get("client_id")
|
||||
if _, err := portalFromCIMDClientID(r.Context(), h.trust, clientID); err == nil {
|
||||
q := url.Values{}
|
||||
q.Set("authorize", r.URL.Query().Encode())
|
||||
loginURL := h.baseURL.WithPath(h.portalLoginPath).WithQueryValues(q).MustString()
|
||||
http.Redirect(w, r, loginURL, http.StatusFound)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
loginURL := h.baseURL.WithPath("/auth/login").
|
||||
WithQuery("continue", continueURL).
|
||||
MustString()
|
||||
|
||||
@@ -21,21 +21,17 @@
|
||||
package connect_v1
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"go.gearno.de/kit/httpserver"
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/baseurl"
|
||||
trust "go.probo.inc/probo/pkg/complianceportal/visitor"
|
||||
"go.probo.inc/probo/pkg/coredata"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/iam"
|
||||
"go.probo.inc/probo/pkg/iam/oauth2"
|
||||
"go.probo.inc/probo/pkg/mail"
|
||||
"go.probo.inc/probo/pkg/saferedirect"
|
||||
"go.probo.inc/probo/pkg/securecookie"
|
||||
@@ -221,7 +217,6 @@ func parseOIDCProvider(s string) (coredata.OIDCProvider, error) {
|
||||
|
||||
type MagicLinkHandler struct {
|
||||
iam *iam.Service
|
||||
trust *trust.Service
|
||||
proboBaseURL *baseurl.BaseURL
|
||||
sessionCookie *authn.Cookie
|
||||
safeRedirect *saferedirect.SafeRedirect
|
||||
@@ -230,17 +225,16 @@ type MagicLinkHandler struct {
|
||||
|
||||
func NewMagicLinkHandler(
|
||||
iamSvc *iam.Service,
|
||||
trustSvc *trust.Service,
|
||||
proboBaseURL *baseurl.BaseURL,
|
||||
cookieConfig securecookie.Config,
|
||||
logger *log.Logger,
|
||||
allowedHost saferedirect.AllowedHostFunc,
|
||||
) *MagicLinkHandler {
|
||||
return &MagicLinkHandler{
|
||||
iam: iamSvc,
|
||||
trust: trustSvc,
|
||||
proboBaseURL: proboBaseURL,
|
||||
sessionCookie: authn.NewCookie(&cookieConfig),
|
||||
safeRedirect: saferedirect.New(nil),
|
||||
safeRedirect: saferedirect.New(allowedHost),
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
@@ -259,17 +253,15 @@ func (h *MagicLinkHandler) SendHandler(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
authorizeContinue := h.authorizeContinueURL(r.FormValue("authorize"))
|
||||
if authorizeContinue == "" {
|
||||
httpserver.RenderError(w, http.StatusBadRequest, errors.New("invalid authorize parameters"))
|
||||
continueParam := r.FormValue("continue")
|
||||
if continueParam == "" {
|
||||
httpserver.RenderError(w, http.StatusBadRequest, errors.New("invalid magic link parameters"))
|
||||
return
|
||||
}
|
||||
|
||||
compliancePageID, organizationID, err := h.portalIDsFromAuthorize(ctx, r.FormValue("authorize"))
|
||||
if err != nil {
|
||||
h.logger.WarnCtx(ctx, "cannot resolve compliance portal from authorize params", log.Error(err))
|
||||
httpserver.RenderError(w, http.StatusBadRequest, errors.New("invalid authorize parameters"))
|
||||
|
||||
safeContinue, ok := h.safeRedirect.Validate(ctx, continueParam)
|
||||
if !ok {
|
||||
httpserver.RenderError(w, http.StatusBadRequest, errors.New("invalid continue URL"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -277,11 +269,13 @@ func (h *MagicLinkHandler) SendHandler(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
req := &iam.SendMagicLinkRequest{
|
||||
Email: emailAddr,
|
||||
CompliancePageID: compliancePageID,
|
||||
OrganizationID: organizationID,
|
||||
URLPath: "/api/connect/v1/magic-link/verify",
|
||||
Continue: &authorizeContinue,
|
||||
MagicLinkBaseURL: &proboURL,
|
||||
Continue: &safeContinue,
|
||||
}
|
||||
|
||||
if clientID := oauth2ClientIDFromContinueURL(safeContinue); clientID != "" {
|
||||
req.OAuth2ClientIDRaw = &clientID
|
||||
}
|
||||
|
||||
if err := h.iam.AuthService.SendMagicLink(ctx, req); err != nil {
|
||||
@@ -342,61 +336,3 @@ func (h *MagicLinkHandler) VerifyHandler(w http.ResponseWriter, r *http.Request)
|
||||
|
||||
http.Redirect(w, r, redirectURL, http.StatusFound)
|
||||
}
|
||||
|
||||
func (h *MagicLinkHandler) authorizeContinueURL(encodedAuthorize string) string {
|
||||
if encodedAuthorize == "" {
|
||||
return ""
|
||||
}
|
||||
|
||||
values, err := url.ParseQuery(encodedAuthorize)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
metadata := OAuth2ServerMetadata(
|
||||
h.proboBaseURL,
|
||||
h.iam.OAuth2ScopeRegistry.RegisteredScopes(),
|
||||
)
|
||||
|
||||
continueURL, err := oauth2.AuthorizationURLWithQuery(metadata.AuthorizationEndpoint, values)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
return continueURL
|
||||
}
|
||||
|
||||
func portalFromCIMDClientID(
|
||||
ctx context.Context,
|
||||
trustSvc *trust.Service,
|
||||
clientID string,
|
||||
) (*coredata.TrustCenter, error) {
|
||||
host, ok := oauth2.CIMDClientIDHost(clientID)
|
||||
if !ok {
|
||||
return nil, errors.New("invalid cimd client_id")
|
||||
}
|
||||
|
||||
portal, err := trustSvc.GetPortalByDomainName(ctx, host)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return portal, nil
|
||||
}
|
||||
|
||||
func (h *MagicLinkHandler) portalIDsFromAuthorize(
|
||||
ctx context.Context,
|
||||
encodedAuthorize string,
|
||||
) (*gid.GID, gid.GID, error) {
|
||||
values, err := url.ParseQuery(encodedAuthorize)
|
||||
if err != nil {
|
||||
return nil, gid.GID{}, err
|
||||
}
|
||||
|
||||
portal, err := portalFromCIMDClientID(ctx, h.trust, values.Get("client_id"))
|
||||
if err != nil {
|
||||
return nil, gid.GID{}, err
|
||||
}
|
||||
|
||||
return &portal.ID, portal.OrganizationID, nil
|
||||
}
|
||||
|
||||
@@ -68,6 +68,7 @@ type (
|
||||
batchAuthorize authz.BatchAuthorizeFunc
|
||||
logger *log.Logger
|
||||
iam *iam.Service
|
||||
trust *trust.Service
|
||||
scopeRegistry *oauth2scope.Registry
|
||||
fileManager *filemanager.Service
|
||||
baseURL *baseurl.BaseURL
|
||||
@@ -92,7 +93,7 @@ func NewMux(
|
||||
apiKeyMiddleware := authn.NewAPIKeyMiddleware(svc, tokenSecret)
|
||||
oauth2Middleware := authn.NewOAuth2AccessTokenMiddleware(svc)
|
||||
identityPresenceMiddleware := authn.NewIdentityPresenceMiddleware(baseURL)
|
||||
graphqlHandler := NewGraphQLHandler(svc, logger, fileManagerSvc, baseURL, cookieConfig, graphqlLimits)
|
||||
graphqlHandler := NewGraphQLHandler(svc, trustSvc, logger, fileManagerSvc, baseURL, cookieConfig, graphqlLimits)
|
||||
samlHandler := NewSAMLHandler(svc, cookieConfig, baseURL, logger)
|
||||
scimHandler := NewSCIMHandler(svc, logger.Named("scim"))
|
||||
|
||||
@@ -106,18 +107,16 @@ func NewMux(
|
||||
|
||||
magicLinkHandler := NewMagicLinkHandler(
|
||||
svc,
|
||||
trustSvc,
|
||||
baseURL,
|
||||
cookieConfig,
|
||||
logger,
|
||||
allowedRedirectHost,
|
||||
)
|
||||
|
||||
oauth2Handler := NewOAuth2Handler(
|
||||
svc,
|
||||
trustSvc,
|
||||
cookieConfig,
|
||||
baseURL,
|
||||
"/auth/portal-login",
|
||||
logger,
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user