Show missing OAuth scopes after connector reconnect
Partial grants completed without feedback, leaving Reconnect required unexplained. Keep the token and toast the backend missing-scopes error after the OAuth callback redirect. Signed-off-by: Sacha Al Himdani <sacha@probo.com>
This commit is contained in:
@@ -47,6 +47,13 @@ import { AccessReviewSourceRow } from "../_components/AccessReviewSourceRow";
|
||||
import { createAccessReviewSourceMutation } from "../dialogs/accessReviewSourceMutations";
|
||||
import { AddAccessReviewSourceDialog, addAccessReviewSourceDialogConnectorProviderInfoFragment } from "../dialogs/AddAccessReviewSourceDialog";
|
||||
|
||||
function clearOAuthCallbackParams(params: URLSearchParams) {
|
||||
params.delete("connector_id");
|
||||
params.delete("provider");
|
||||
params.delete("error");
|
||||
return params;
|
||||
}
|
||||
|
||||
export const accessReviewSourcesTabQuery = graphql`
|
||||
query AccessReviewSourcesTabQuery($organizationId: ID!) {
|
||||
accessReviewDrivers {
|
||||
@@ -145,9 +152,11 @@ export default function AccessReviewSourcesTab({ queryRef }: Props) {
|
||||
);
|
||||
|
||||
// Handle OAuth callback: after the provider redirects back with connector_id,
|
||||
// automatically create the access source for that connector.
|
||||
// automatically create the access source for that connector. Missing scopes
|
||||
// arrive as a backend error query param and are toasted like other errors.
|
||||
const callbackConnectorId = searchParams.get("connector_id");
|
||||
const callbackProvider = searchParams.get("provider");
|
||||
const callbackError = searchParams.get("error");
|
||||
const hasSourceForCallback = !!callbackConnectorId
|
||||
&& accessReviewSources?.edges.some(edge => edge.node.connectorId === callbackConnectorId);
|
||||
|
||||
@@ -155,11 +164,22 @@ export default function AccessReviewSourcesTab({ queryRef }: Props) {
|
||||
if (!callbackConnectorId) return;
|
||||
|
||||
if (hasSourceForCallback) {
|
||||
setSearchParams((params) => {
|
||||
params.delete("connector_id");
|
||||
params.delete("provider");
|
||||
return params;
|
||||
}, { replace: true });
|
||||
// Create sets processedConnectorIdRef before the mutation; when Relay
|
||||
// inserts the edge mid-callback, skip toasting here so onCompleted is
|
||||
// the only toast. Reconnect never sets that ref, so it still toasts.
|
||||
const createInFlight
|
||||
= processedConnectorIdRef.current === callbackConnectorId;
|
||||
if (callbackError && !createInFlight) {
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.error"),
|
||||
description: callbackError,
|
||||
variant: "error",
|
||||
});
|
||||
}
|
||||
if (!createInFlight) {
|
||||
processedConnectorIdRef.current = null;
|
||||
setSearchParams(clearOAuthCallbackParams, { replace: true });
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -186,11 +206,7 @@ export default function AccessReviewSourcesTab({ queryRef }: Props) {
|
||||
onCompleted(_, errors) {
|
||||
if (errors?.length) {
|
||||
processedConnectorIdRef.current = null;
|
||||
setSearchParams((params) => {
|
||||
params.delete("connector_id");
|
||||
params.delete("provider");
|
||||
return params;
|
||||
}, { replace: true });
|
||||
setSearchParams(clearOAuthCallbackParams, { replace: true });
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.error"),
|
||||
description: formatError(
|
||||
@@ -201,24 +217,25 @@ export default function AccessReviewSourcesTab({ queryRef }: Props) {
|
||||
});
|
||||
return;
|
||||
}
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.success"),
|
||||
description: t("accessReviewSourcesTab.messages.created"),
|
||||
variant: "success",
|
||||
});
|
||||
setSearchParams((params) => {
|
||||
params.delete("connector_id");
|
||||
params.delete("provider");
|
||||
return params;
|
||||
}, { replace: true });
|
||||
if (callbackError) {
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.error"),
|
||||
description: callbackError,
|
||||
variant: "error",
|
||||
});
|
||||
} else {
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.success"),
|
||||
description: t("accessReviewSourcesTab.messages.created"),
|
||||
variant: "success",
|
||||
});
|
||||
}
|
||||
processedConnectorIdRef.current = null;
|
||||
setSearchParams(clearOAuthCallbackParams, { replace: true });
|
||||
},
|
||||
onError(error) {
|
||||
processedConnectorIdRef.current = null;
|
||||
setSearchParams((params) => {
|
||||
params.delete("connector_id");
|
||||
params.delete("provider");
|
||||
return params;
|
||||
}, { replace: true });
|
||||
setSearchParams(clearOAuthCallbackParams, { replace: true });
|
||||
toast({
|
||||
title: t("accessReviewSourcesTab.messages.error"),
|
||||
description: formatError(
|
||||
@@ -232,6 +249,7 @@ export default function AccessReviewSourcesTab({ queryRef }: Props) {
|
||||
}, [
|
||||
callbackConnectorId,
|
||||
callbackProvider,
|
||||
callbackError,
|
||||
connectorProviderInfos,
|
||||
createAccessReviewSource,
|
||||
hasSourceForCallback,
|
||||
|
||||
@@ -23,6 +23,7 @@ package accessreview
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
)
|
||||
@@ -34,6 +35,7 @@ var (
|
||||
ErrCampaignNotPendingActions = errors.New("access review campaign not pending actions")
|
||||
ErrCampaignCompleted = errors.New("access review campaign completed")
|
||||
ErrCampaignCancelled = errors.New("access review campaign cancelled")
|
||||
ErrMissingOAuthScopes = errors.New("missing required OAuth scopes")
|
||||
)
|
||||
|
||||
type (
|
||||
@@ -60,6 +62,10 @@ type (
|
||||
CampaignCancelledError struct {
|
||||
CampaignID gid.GID
|
||||
}
|
||||
|
||||
MissingOAuthScopesError struct {
|
||||
Scopes []string
|
||||
}
|
||||
)
|
||||
|
||||
func NewCampaignMissingSourcesError(campaignID gid.GID) error {
|
||||
@@ -142,3 +148,20 @@ func (e *CampaignCancelledError) Error() string {
|
||||
func (e *CampaignCancelledError) Is(target error) bool {
|
||||
return target == ErrCampaignCancelled
|
||||
}
|
||||
|
||||
func NewMissingOAuthScopesError(scopes []string) error {
|
||||
return &MissingOAuthScopesError{Scopes: append([]string(nil), scopes...)}
|
||||
}
|
||||
|
||||
func (e *MissingOAuthScopesError) Error() string {
|
||||
display := make([]string, len(e.Scopes))
|
||||
for i, scope := range e.Scopes {
|
||||
display[i] = strings.TrimPrefix(scope, "https://graph.microsoft.com/")
|
||||
}
|
||||
|
||||
return "Missing required OAuth scopes: " + strings.Join(display, ", ")
|
||||
}
|
||||
|
||||
func (e *MissingOAuthScopesError) Is(target error) bool {
|
||||
return target == ErrMissingOAuthScopes
|
||||
}
|
||||
|
||||
@@ -98,3 +98,19 @@ func TestCampaignClientErrors(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMissingOAuthScopesError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := accessreview.NewMissingOAuthScopesError([]string{
|
||||
"https://graph.microsoft.com/AuditLog.Read.All",
|
||||
"openid",
|
||||
})
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
"Missing required OAuth scopes: AuditLog.Read.All, openid",
|
||||
err.Error(),
|
||||
)
|
||||
assert.ErrorIs(t, err, accessreview.ErrMissingOAuthScopes)
|
||||
}
|
||||
|
||||
112
pkg/accessreview/source_missing_scopes_test.go
Normal file
112
pkg/accessreview/source_missing_scopes_test.go
Normal file
@@ -0,0 +1,112 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission is hereby granted, free of charge, to any person obtaining a copy
|
||||
// of this software and associated documentation files (the "Software"), to deal
|
||||
// in the Software without restriction, including without limitation the rights
|
||||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
// copies of the Software, and to permit persons to whom the Software is
|
||||
// furnished to do so, subject to the following conditions:
|
||||
//
|
||||
// The above copyright notice and this permission notice shall be included in
|
||||
// all copies or substantial portions of the Software.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
||||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
|
||||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
|
||||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
|
||||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
|
||||
// SOFTWARE.
|
||||
|
||||
package accessreview
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"go.probo.inc/probo/pkg/connector"
|
||||
"go.probo.inc/probo/pkg/coredata"
|
||||
)
|
||||
|
||||
func TestMissingOAuthScopesForConnector(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
required := []string{
|
||||
"openid",
|
||||
"https://graph.microsoft.com/AuditLog.Read.All",
|
||||
"https://graph.microsoft.com/User.Read.All",
|
||||
}
|
||||
|
||||
t.Run("non oauth protocol returns empty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dbConnector := coredata.Connector{
|
||||
Protocol: coredata.ConnectorProtocolAPIKey,
|
||||
Connection: &connector.APIKeyConnection{APIKey: "k"},
|
||||
}
|
||||
|
||||
assert.Empty(t, missingOAuthScopesForConnector(dbConnector, required))
|
||||
})
|
||||
|
||||
t.Run("empty required returns empty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dbConnector := coredata.Connector{
|
||||
Protocol: coredata.ConnectorProtocolOAuth2,
|
||||
Connection: &connector.OAuth2Connection{
|
||||
Scope: "openid",
|
||||
},
|
||||
}
|
||||
|
||||
assert.Empty(t, missingOAuthScopesForConnector(dbConnector, nil))
|
||||
})
|
||||
|
||||
t.Run("nil connection treats grant as empty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dbConnector := coredata.Connector{
|
||||
Protocol: coredata.ConnectorProtocolOAuth2,
|
||||
Connection: nil,
|
||||
}
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
[]string{
|
||||
"https://graph.microsoft.com/AuditLog.Read.All",
|
||||
"https://graph.microsoft.com/User.Read.All",
|
||||
"openid",
|
||||
},
|
||||
missingOAuthScopesForConnector(dbConnector, required),
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("partial grant returns missing scopes", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dbConnector := coredata.Connector{
|
||||
Protocol: coredata.ConnectorProtocolOAuth2,
|
||||
Connection: &connector.OAuth2Connection{
|
||||
Scope: "openid User.Read.All",
|
||||
},
|
||||
}
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
[]string{"https://graph.microsoft.com/AuditLog.Read.All"},
|
||||
missingOAuthScopesForConnector(dbConnector, required),
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("full grant returns empty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dbConnector := coredata.Connector{
|
||||
Protocol: coredata.ConnectorProtocolOAuth2,
|
||||
Connection: &connector.OAuth2Connection{
|
||||
Scope: "openid AuditLog.Read.All User.Read.All",
|
||||
},
|
||||
}
|
||||
|
||||
assert.Empty(t, missingOAuthScopesForConnector(dbConnector, required))
|
||||
})
|
||||
}
|
||||
@@ -582,17 +582,17 @@ func (s *Service) SourceNeedsConfiguration(
|
||||
return cfg.SelectedSlug(dbConnector) == "", nil
|
||||
}
|
||||
|
||||
// SourceNeedsReconnect reports whether the connector is missing OAuth scopes
|
||||
// required by the current provider registration. Only OAuth2 connectors are
|
||||
// checked: API-key (and other non-OAuth) credentials have no grant scopes and
|
||||
// cannot be repaired by an OAuth reconnect, even when the provider also
|
||||
// advertises OAuth2Scopes for its dual-auth path. ErrResourceNotFound is
|
||||
// propagated for a missing connector.
|
||||
func (s *Service) SourceNeedsReconnect(
|
||||
// SourceMissingOAuthScopes returns the OAuth scopes required by the current
|
||||
// provider registration that are absent from the connector's stored grant.
|
||||
// Only OAuth2 connectors are checked: API-key (and other non-OAuth)
|
||||
// credentials have no grant scopes and return an empty slice, even when the
|
||||
// provider also advertises OAuth2Scopes for its dual-auth path.
|
||||
// ErrResourceNotFound is propagated for a missing connector.
|
||||
func (s *Service) SourceMissingOAuthScopes(
|
||||
ctx context.Context,
|
||||
scope coredata.Scoper,
|
||||
connectorID gid.GID,
|
||||
) (bool, error) {
|
||||
) ([]string, error) {
|
||||
var dbConnector coredata.Connector
|
||||
|
||||
err := s.pg.WithConn(
|
||||
@@ -606,23 +606,52 @@ func (s *Service) SourceNeedsReconnect(
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
if dbConnector.Protocol != coredata.ConnectorProtocolOAuth2 {
|
||||
return false, nil
|
||||
return nil, err
|
||||
}
|
||||
|
||||
required := s.providerRegistry.ProviderOAuth2Scopes(dbConnector.Provider)
|
||||
|
||||
return missingOAuthScopesForConnector(dbConnector, required), nil
|
||||
}
|
||||
|
||||
// SourceNeedsReconnect reports whether the connector is missing OAuth scopes
|
||||
// required by the current provider registration. ErrResourceNotFound is
|
||||
// propagated for a missing connector.
|
||||
func (s *Service) SourceNeedsReconnect(
|
||||
ctx context.Context,
|
||||
scope coredata.Scoper,
|
||||
connectorID gid.GID,
|
||||
) (bool, error) {
|
||||
missing, err := s.SourceMissingOAuthScopes(ctx, scope, connectorID)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
return len(missing) > 0, nil
|
||||
}
|
||||
|
||||
// missingOAuthScopesForConnector returns scopes in required that are absent
|
||||
// from the connector's stored OAuth grant. Non-OAuth connectors and empty
|
||||
// required lists yield an empty result. A nil Connection is treated as
|
||||
// granting nothing.
|
||||
func missingOAuthScopesForConnector(
|
||||
dbConnector coredata.Connector,
|
||||
required []string,
|
||||
) []string {
|
||||
if dbConnector.Protocol != coredata.ConnectorProtocolOAuth2 {
|
||||
return []string{}
|
||||
}
|
||||
|
||||
if len(required) == 0 {
|
||||
return false, nil
|
||||
return []string{}
|
||||
}
|
||||
|
||||
if dbConnector.Connection == nil {
|
||||
return true, nil
|
||||
var granted []string
|
||||
if dbConnector.Connection != nil {
|
||||
granted = dbConnector.Connection.Scopes()
|
||||
}
|
||||
|
||||
return len(connector.MissingScopes(required, dbConnector.Connection.Scopes())) > 0, nil
|
||||
return connector.MissingScopes(required, granted)
|
||||
}
|
||||
|
||||
// AutoSelectDefaultOrganization picks the first workspace/org a freshly linked
|
||||
|
||||
@@ -28,6 +28,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"go.gearno.de/kit/httpserver"
|
||||
@@ -431,6 +432,19 @@ func handleConnectorComplete(
|
||||
q := parsedURL.Query()
|
||||
q.Set("connector_id", cnnctr.ID.String())
|
||||
q.Set("provider", string(connectorProvider))
|
||||
|
||||
// Access-review sources toast missing scopes after redirect. Other
|
||||
// continue URLs (Slack compliance page, SCIM settings, …) must not
|
||||
// get a false missing-scope error from this access-review check.
|
||||
if strings.Contains(state.ContinueURL, "/access-reviews/sources") {
|
||||
missing, err := accessReviewSvc.SourceMissingOAuthScopes(r.Context(), scope, cnnctr.ID)
|
||||
if err != nil {
|
||||
logger.WarnCtx(r.Context(), "cannot determine missing OAuth scopes after connector callback", log.Error(err))
|
||||
} else if len(missing) > 0 {
|
||||
q.Set("error", accessreview.NewMissingOAuthScopesError(missing).Error())
|
||||
}
|
||||
}
|
||||
|
||||
parsedURL.RawQuery = q.Encode()
|
||||
|
||||
safeRedirect.Redirect(w, r, parsedURL.String(), "/", http.StatusSeeOther)
|
||||
|
||||
Reference in New Issue
Block a user