Skip to content

Commit a6eaf19

Browse files
committed
feat(coderd): implement OIDC revocation with RP initiated logout
1 parent 91c274a commit a6eaf19

File tree

2 files changed

+108
-20
lines changed

2 files changed

+108
-20
lines changed

coderd/userauth.go

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package coderd
33
import (
44
"context"
55
"database/sql"
6+
"encoding/base64"
67
"errors"
78
"fmt"
9+
"io"
810
"net/http"
911
"net/mail"
1012
"net/url"
@@ -735,7 +737,78 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) {
735737
})
736738
}
737739

738-
// Returns URL for the OIDC logout.
740+
// getDiscoveryEndpoints will return endpoints for end session and revocation
741+
func (api *API) getDiscoveryEndpoints() (endSessionEndpoint string, revocationEndpoint string, err error) {
742+
oidcProvider := api.OIDCConfig.Provider
743+
744+
var discoveryConfig struct {
745+
EndSessionEndpoint string `json:"end_session_endpoint"`
746+
RevocationEndpoint string `json:"revocation_endpoint"`
747+
}
748+
749+
// Extract endpoints
750+
if err := oidcProvider.Claims(&discoveryConfig); err != nil {
751+
return "", "", xerrors.Errorf("failed to extract endpoints from OIDC provider discovery claims: %w", err)
752+
}
753+
754+
return discoveryConfig.EndSessionEndpoint, discoveryConfig.RevocationEndpoint, nil
755+
}
756+
757+
// revokeOAuthToken will revoke a particular token
758+
func (api *API) revokeOAuthToken(ctx context.Context, token string, revocationEndpoint string) error {
759+
logger := api.Logger.Named(userAuthLoggerName)
760+
761+
if token == "" || revocationEndpoint == "" {
762+
logger.Warn(ctx, "skip OAuth token revocation")
763+
return nil
764+
}
765+
766+
dvOIDC := api.DeploymentValues.OIDC
767+
oidcClientID := dvOIDC.ClientID.Value()
768+
oidcClientSecret := dvOIDC.ClientSecret.Value()
769+
770+
if oidcClientID == "" || oidcClientSecret == "" {
771+
return xerrors.New("missing required configs for revocation (endpoint, client ID, or secret)")
772+
}
773+
774+
data := url.Values{}
775+
data.Set("token", token)
776+
777+
revokeReq, err := http.NewRequestWithContext(ctx, http.MethodPost, revocationEndpoint, strings.NewReader(data.Encode()))
778+
if err != nil {
779+
return xerrors.Errorf("failed to create revoke request object: %w", err)
780+
}
781+
782+
revokeReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
783+
auth := base64.StdEncoding.EncodeToString([]byte(oidcClientID + ":" + oidcClientSecret))
784+
revokeReq.Header.Set("Authorization", "Basic "+auth)
785+
786+
httpClient := &http.Client{}
787+
resp, err := httpClient.Do(revokeReq)
788+
if err != nil {
789+
return xerrors.Errorf("failed to send revoke request to %s: %w", revocationEndpoint, err)
790+
}
791+
defer resp.Body.Close()
792+
793+
if resp.StatusCode != http.StatusOK {
794+
respBodyBytes, _ := io.ReadAll(resp.Body)
795+
respBodyStr := string(respBodyBytes)
796+
797+
logger.Warn(ctx, "failed to request OAuth token revocation",
798+
slog.F("status_code", resp.StatusCode),
799+
slog.F("response_body", respBodyStr),
800+
slog.F("endpoint", revocationEndpoint),
801+
slog.F("client_id", oidcClientID),
802+
)
803+
804+
return xerrors.Errorf("failed to revoke with status %d: %s", resp.StatusCode, respBodyStr)
805+
}
806+
807+
logger.Info(ctx, "success to revoke OAuth token", slog.F("status_code", resp.StatusCode))
808+
return nil // Success
809+
}
810+
811+
// Returns URL for the OIDC logout after token revocation.
739812
//
740813
// @Summary Get user OIDC logout URL
741814
// @ID get-user-oidc-logout-url
@@ -745,8 +818,18 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) {
745818
// @Success 200 {object} codersdk.OIDCLogoutResponse "Returns a map containing the OIDC logout URL"
746819
// @Router /users/oidc-logout [get]
747820
func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) {
821+
logger := api.Logger.Named(userAuthLoggerName)
748822
ctx := r.Context()
749823

824+
// Check if OIDC is configured
825+
if api.OIDCConfig == nil || api.OIDCConfig.Provider == nil {
826+
logger.Warn(ctx, "unable to support OIDC logout with current configuration")
827+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
828+
Message: "Failed to retrieve OIDC configuration.",
829+
})
830+
return
831+
}
832+
750833
// Get logged-in user
751834
apiKey := httpmw.APIKey(r)
752835
user, err := api.Database.GetUserByID(ctx, apiKey.UserID)
@@ -757,8 +840,6 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) {
757840
return
758841
}
759842

760-
logger := api.Logger.Named(userAuthLoggerName)
761-
762843
// Default response: empty URL if OIDC logout is not supported
763844
response := codersdk.OIDCLogoutResponse{URL: ""}
764845

@@ -784,22 +865,39 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) {
784865
return
785866
}
786867

787-
rawIDToken := link.OAuthAccessToken
868+
accessToken := link.OAuthAccessToken
869+
refreshToken := link.OAuthRefreshToken
788870

789871
// Retrieve OIDC environment variables
790872
dvOIDC := api.DeploymentValues.OIDC
791-
oidcEndpoint := dvOIDC.LogoutEndpoint.Value()
792873
oidcClientID := dvOIDC.ClientID.Value()
793874
logoutURI := dvOIDC.LogoutRedirectURI.Value()
794875

795-
if oidcEndpoint == "" {
876+
endSessionEndpoint, revocationEndpoint, err := api.getDiscoveryEndpoints()
877+
if err != nil {
878+
logger.Error(ctx, "failed to get OIDC discovery endpoints", slog.Error(err))
879+
880+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
881+
Message: "Failed to process OIDC configuration.",
882+
})
883+
return
884+
}
885+
886+
// Perform token revocation first
887+
err = api.revokeOAuthToken(ctx, refreshToken, revocationEndpoint)
888+
if err != nil {
889+
// Do not return since this step is optional
890+
logger.Warn(ctx, "failed to revoke OAuth token during logout", slog.Error(err))
891+
}
892+
893+
if endSessionEndpoint == "" {
796894
logger.Warn(ctx, "missing OIDC logout endpoint")
797895
httpapi.Write(ctx, rw, http.StatusOK, response)
798896
return
799897
}
800898

801899
// Construct OIDC Logout URL
802-
logoutURL, err := url.Parse(oidcEndpoint)
900+
logoutURL, err := url.Parse(endSessionEndpoint)
803901
if err != nil {
804902
logger.Error(ctx, "failed to parse OIDC endpoint", "error", err)
805903
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -812,12 +910,12 @@ func (api *API) userOIDCLogoutURL(rw http.ResponseWriter, r *http.Request) {
812910
// Build parameters
813911
q := url.Values{}
814912

913+
if accessToken != "" {
914+
q.Set("id_token_hint", accessToken)
915+
}
815916
if oidcClientID != "" {
816917
q.Set("client_id", oidcClientID)
817918
}
818-
if rawIDToken != "" {
819-
q.Set("id_token_hint", rawIDToken)
820-
}
821919
if logoutURI != "" {
822920
q.Set("logout_uri", logoutURI)
823921
}

codersdk/deployment.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,16 +1919,6 @@ func (c *DeploymentValues) Options() serpent.OptionSet {
19191919
Group: &deploymentGroupOIDC,
19201920
YAML: "dangerousSkipIssuerChecks",
19211921
},
1922-
{
1923-
Name: "OIDC logout endpoint",
1924-
Description: "OIDC endpoint for logout.",
1925-
Flag: "logout-endpoint",
1926-
Env: "CODER_OIDC_LOGOUT_ENDPOINT",
1927-
Default: "",
1928-
Value: &c.OIDC.LogoutEndpoint,
1929-
Group: &deploymentGroupOIDC,
1930-
YAML: "logoutEndpoint",
1931-
},
19321922
{
19331923
Name: "OIDC logout redirect URI",
19341924
Description: "OIDC redirect URI after logout.",

0 commit comments

Comments
 (0)