diff --git a/Dockerfile.e2e b/Dockerfile.e2e new file mode 100644 index 0000000..b44756a --- /dev/null +++ b/Dockerfile.e2e @@ -0,0 +1,97 @@ +ARG CRYSTAL_VERSION=latest + +FROM placeos/crystal:$CRYSTAL_VERSION AS build +WORKDIR /app + +# Set the commit via a build arg +ARG PLACE_COMMIT="DEV" +# Set the platform version via a build arg +ARG PLACE_VERSION="DEV" + +# Create a non-privileged user, defaults are appuser:10001 +ARG IMAGE_UID="10001" +ENV UID=$IMAGE_UID +ENV USER=appuser + +# See https://stackoverflow.com/a/55757473/12429735 +RUN adduser \ + --disabled-password \ + --gecos "" \ + --home "/nonexistent" \ + --shell "/sbin/nologin" \ + --no-create-home \ + --uid "${UID}" \ + "${USER}" + +# Install package updates since image release +RUN apk update && apk --no-cache --quiet upgrade + +RUN update-ca-certificates + +# Install shards for caching +COPY shard.yml shard.yml +COPY shard.override.yml shard.override.yml +COPY shard.lock shard.lock + +RUN shards install --production --ignore-crystal-version --skip-postinstall --skip-executables + +# Add src +COPY ./src /app/src + +# Build application +RUN UNAME_AT_COMPILE_TIME=true \ + PLACE_COMMIT=$PLACE_COMMIT \ + PLACE_VERSION=$PLACE_VERSION \ + shards build --production --error-trace --static + +SHELL ["/bin/ash", "-eo", "pipefail", "-c"] + +# Extract binary dependencies +RUN mkdir -p /app/deps && for binary in /app/bin/*; do \ + { ldd "$binary" 2>/dev/null || true; } | \ + tr -s '[:blank:]' '\n' | \ + grep '^/' | \ + xargs -I % sh -c 'mkdir -p $(dirname deps%); cp % deps%;' || true; \ + done + +# Generate OpenAPI docs while we still have source code access +RUN ./bin/placeos-auth --docs > openapi.yml + +RUN git config --system http.sslCAInfo /etc/ssl/certs/ca-certificates.crt + +# Build a minimal docker image +FROM scratch +WORKDIR / +ENV PATH=$PATH:/ + +# Copy the user information over +COPY --from=build etc/passwd /etc/passwd +COPY --from=build /etc/group /etc/group + +# These are required for communicating with external services +COPY --from=build /etc/hosts /etc/hosts + +# These provide certificate chain validation where communicating with external services over TLS +COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ +ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt + +# This is required for Timezone support +COPY --from=build /usr/share/zoneinfo/ /usr/share/zoneinfo/ + +# This is your application +COPY --from=build /app/deps / +COPY --from=build /app/bin / + +# Copy the docs into the container, you can serve this file in your app +COPY --from=build /app/openapi.yml /openapi.yml + +# Use an unprivileged user. +USER appuser:appuser + +# Spider-gazelle has a built in helper for health checks +HEALTHCHECK CMD ["/placeos-auth", "-c", "http://127.0.0.1:8080/auth/healthz"] + +# Run the app binding on port 8080 +EXPOSE 8080 +ENTRYPOINT ["/placeos-auth"] +CMD ["/placeos-auth", "-b", "0.0.0.0", "-p", "8080"] diff --git a/docker-compose.override.yml b/docker-compose.override.yml new file mode 100644 index 0000000..068fbe5 --- /dev/null +++ b/docker-compose.override.yml @@ -0,0 +1,11 @@ +# LOCAL-ONLY (untracked): remap published test-DB ports off the host's +# native postgres:5432 / redis:6379 so the isolated spec stack can start. +# The `test` service still reaches these over the compose network by +# hostname, so connectivity is unchanged. +services: + postgres: + ports: + - "15499:5432" + redis: + ports: + - "16399:6379" diff --git a/spec/controllers/oauth_provider_flows_spec.cr b/spec/controllers/oauth_provider_flows_spec.cr index 405e5a0..36ff8eb 100644 --- a/spec/controllers/oauth_provider_flows_spec.cr +++ b/spec/controllers/oauth_provider_flows_spec.cr @@ -140,6 +140,35 @@ module PlaceOS::Auth cleanup_login.call(uid) if uid strat.try &.destroy end + + it "recovers the strat id from the session when the IdP drops ?id= on the callback" do + strat = google_strat.call + uid = "google-noid-#{Random.rand(999999)}" + email = "grace-#{Random.rand(999999)}@localhost" + + WebMock.stub(:post, "https://accounts.google.com/token").to_return( + status: 200, headers: json_headers, + body: {access_token: "g-access", token_type: "Bearer", expires_in: 3600}.to_json, + ) + WebMock.stub(:get, "https://openidconnect.googleapis.com/v1/userinfo").to_return( + status: 200, headers: json_headers, + body: {sub: uid, email: email, name: "Grace Hopper", given_name: "Grace", family_name: "Hopper"}.to_json, + ) + + k = kickoff.call(strat.id.as(String)) + # The callback URL deliberately OMITS `?id=` (some IdPs don't round-trip + # it). The id must be recovered from the session state stashed at kickoff + # — otherwise this 401s "oauth state mismatch". + result = client.get( + "/auth/oauth2/callback?code=g-code&state=#{k[:state]}", + headers: HTTP::Headers{"Host" => "localhost", "Cookie" => k[:cookie]}, + ) + result.status_code.should eq 303 + ::PlaceOS::Model::UserAuthLookup.find?("auth-#{authority_id.call}-oauth2-#{uid}").should_not be_nil + ensure + cleanup_login.call(uid) if uid + strat.try &.destroy + end end # ---- Azure AD / Entra ID ----------------------------------------- diff --git a/spec/controllers/saml_callbacks_spec.cr b/spec/controllers/saml_callbacks_spec.cr index 3a0d3e2..ef6ad41 100644 --- a/spec/controllers/saml_callbacks_spec.cr +++ b/spec/controllers/saml_callbacks_spec.cr @@ -109,5 +109,39 @@ module PlaceOS::Auth result.status_code.should eq 404 end end + + # The callback's session-state CSRF check is skipped for SAML — the + # cross-site HTTP-POST binding drops the SameSite=Lax cookie so the stashed + # state is unavailable, and the signed assertion authenticates instead + # (PPT-2536). It stays fully enforced for the OAuth2 path. + describe "callback state check" do + it "does NOT reject a SAML callback as an 'oauth state mismatch'" do + strat = create_saml_strat.call + # No prior kickoff => no stored session state. An OAuth2 callback would + # 401 "oauth state mismatch" here; a SAML callback must get PAST that + # check (and instead fail later at signed-assertion validation). + result = client.post( + "/auth/saml/callback?id=#{URI.encode_www_form(strat.id.as(String))}", + headers: HTTP::Headers{ + "Host" => "localhost", + "Content-Type" => "application/x-www-form-urlencoded", + }, + body: "SAMLResponse=#{URI.encode_www_form("not-a-real-assertion")}&RelayState=xyz", + ) + result.body.should_not contain "oauth state mismatch" + ensure + strat.try &.destroy + end + + it "still enforces the session-state check for an OAuth2 callback" do + # No stored state => the OAuth2 path rejects with a state mismatch. + result = client.get( + "/auth/oauth2/callback?id=whatever&state=whatever", + headers: HTTP::Headers{"Host" => "localhost"}, + ) + result.status_code.should eq 401 + result.body.should contain "oauth state mismatch" + end + end end end diff --git a/src/placeos-auth/controllers/provider_callbacks.cr b/src/placeos-auth/controllers/provider_callbacks.cr index 403f7f6..9db2634 100644 --- a/src/placeos-auth/controllers/provider_callbacks.cr +++ b/src/placeos-auth/controllers/provider_callbacks.cr @@ -98,9 +98,33 @@ module PlaceOS::Auth raise Error::NotFound.new("authority not found") if authority.nil? expect_state, expect_provider, expect_id = consume_stored_state - if expect_state.nil? || state != expect_state || expect_provider != provider || expect_id != (id || "") - Log.warn { {action: "provider_callbacks.callback", message: "state mismatch", provider: provider} } - raise Error::Unauthorized.new("oauth state mismatch") + + # Recover the strategy id from the stored session state when the IdP + # dropped the `?id=` query param on the callback redirect (some OAuth + # providers don't round-trip query params on `redirect_uri`). The exact + # id was stashed in `#initiate`, so use it rather than 401 — more precise + # than the legacy Ruby guess (first strat / authority callback URI). A + # present-but-mismatched id is left alone, so the state check below still + # rejects it as CSRF. + if (id.nil? || id.empty?) && (recovered_id = expect_id.presence) + id = recovered_id + end + + # SAML (adfs) uses the HTTP-POST binding: the IdP auto-submits the signed + # assertion cross-site to the ACS URL, so the browser withholds our + # SameSite=Lax auth-flow cookie and the CSRF `state` stashed in + # `#initiate` is unavailable here — and the IdP echoes the CSRF value + # back as `RelayState`, not `state`, anyway. The legacy Ruby service + # (omniauth-saml) never did session-state CSRF for SAML; the callback was + # authenticated purely by the signed assertion (idp_cert / + # idp_cert_fingerprint, want_assertions_signed). Mirror that: skip the + # session-state check for SAML and let `engine.user` validate the + # signature. The OAuth2 path (Lax cookie sent) keeps full validation. + unless saml_provider?(provider) + if expect_state.nil? || state != expect_state || expect_provider != provider || expect_id != (id || "") + Log.warn { {action: "provider_callbacks.callback", message: "state mismatch", provider: provider} } + raise Error::Unauthorized.new("oauth state mismatch") + end end redirect_uri = callback_uri(provider, id) @@ -187,6 +211,15 @@ module PlaceOS::Auth authorize_uri.gsub("%3Fid%3D", "%2F").gsub("?id=", "/") end + # SAML callbacks arrive on the `adfs`/`saml` provider names registered by + # `ExternalProviders`. They cannot participate in session-state CSRF (the + # cross-site HTTP-POST binding drops the SameSite=Lax cookie) and are + # authenticated by the signed assertion instead. + private def saml_provider?(provider : String) : Bool + provider == ExternalProviders::SAML_PROVIDER || + provider == ExternalProviders::SAML_PROVIDER_ALIAS + end + private def consume_stored_state : Tuple(String?, String?, String?) raw = session.delete(SESSION_OAUTH_STATE) return {nil, nil, nil} unless raw.is_a?(String)