Skip to content

fix: add issuer validation #2322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cemalkilic wants to merge 13 commits into from
Closed

fix: add issuer validation #2322

cemalkilic wants to merge 13 commits into from

Conversation

@cemalkilic
Copy link
Contributor

@cemalkilic cemalkilic commented Jan 9, 2026

fix: add issuer validation

jnschaeffer and others added 13 commits December 9, 2025 10:50
@jnschaeffer
feat: Treat rate limit header value as comma-separated list (#2282)
5f2e279 
## What kind of change does this PR introduce?

This PR updates `performRateLimiting` to treat the rate limit header
value as a comma-separated list and enforce rate limiting based on the
first value in that list.

## What is the current behavior?

Certain HTTP headers, such as `X-Forwarded-For` and other headers that
are combined according to RFC 7230, can be represented as a
comma-separated list of values. Intermediate proxies may add their own
values to these headers, modifying the resulting value. For example, an
end user with a single IP address proxied through a fleet of load
balancers using the X-Forwarded-For header may be associated with
multiple `X-Forwarded-For` header values, e.g.,
`2.2.2.2,100.100.100.100` and `2.2.2.2,300.300.300.300`. The current
implementation of `performRateLimiting` treats each of these as separate
rate limiting keys.

## What is the new behavior?

This PR splits the rate limit header by commas and takes the first value
(with whitespace removed) to use as the rate limiting key.

Note that this logic is superficially similar to the
`utilities.GetIPAddress` function with two key differences. In
`performRateLimiting`, there is no set format for a given rate limiting
key, nor is there a fallback value after the first value in the list
that the API should use.
@cstockton
chore: expand e2e phone flow tests and rename helpers (#2288)
db40e28 
Increase tests coverage and verify phone change functionality in
response to supabase/supabase#40797

- Add defensive change to anonymous rate limits test
  - This function relies on low anon rate limit (loops over rate limit)
- Rename getAccessToken to getEmailAccessToken and add generic
getAccessToken helper
- Rename signupAndConfirm to signupAndConfirmEmail
- Expand phone signup and phone change e2e tests:
  - Capture and verify OTP from SendSMS hook
  - Validate one-time token creation and user state transitions
  - Add full phone change flow, including OTP verification
- Update MFA-related tests to use new helper names

Co-authored-by: Chris Stockton <[email protected]>
@siennathesane @cstockton
Fixing bootstrapping issues with migrations. (#2047)
7665a42 
## What kind of change does this PR introduce?

Big fix for #1729, #1848, #1983, and #2040 with an additional type fix.

## What is the current behavior?

The auth service cannot be deployed in a net new environment on
PostgreSQL 17.

## What is the new behavior?

The service is running properly with PostgreSQL 17 in a cleanroom
environment.

## Additional context

Here is a redacted version of the terraform I used to deploy it with. I
used my own container build with these fixes,
`ghcr.io/siennathesane/auth:v2.175.0`, that you can use to verify the
fix is valid, if you want.

```hcl
locals {
  f2-auth-db-namespace = "auth"
}

resource "kubernetes_service_account" "f2-auth" {
  metadata {
    name      = "f2-auth"
    namespace = var.namespace
  }
}

resource "kubernetes_manifest" "f2-auth-db" {
  manifest = {
    "apiVersion" = "postgresql.cnpg.io/v1"
    "kind"       = "Database"
    "metadata" = {
      "name"      = "f2-auth-db"
      "namespace" = var.namespace
    }
    "spec" = {
      "cluster" = {
        "name" =  kubernetes_manifest.f2-cluster.object.metadata.name
      }
      "allowConnections" = true
      "name"             = local.f2-auth-db-namespace
      "owner"            = kubernetes_secret_v1.f2-auth-db.data.username
      "schemas" = [{
        "name"  = local.f2-auth-db-namespace
        "owner" = kubernetes_secret_v1.f2-auth-db.data.username
      }]
    }
  }
}

resource "kubernetes_config_map_v1" "f2-auth-initdb" {
  metadata {
    name      = "sql-commands"
    namespace = var.namespace
  }

  data = {
    "script.sql" = <<-EOT
    ALTER USER ${kubernetes_secret_v1.f2-auth-db.data.username} WITH LOGIN CREATEROLE CREATEDB REPLICATION BYPASSRLS;
    GRANT ${kubernetes_secret_v1.f2-auth-db.data.username} TO postgres;
    CREATE SCHEMA IF NOT EXISTS ${local.f2-auth-db-namespace} AUTHORIZATION ${kubernetes_secret_v1.f2-auth-db.data.username};
    GRANT CREATE ON DATABASE postgres TO ${kubernetes_secret_v1.f2-auth-db.data.username};
    ALTER USER ${kubernetes_secret_v1.f2-auth-db.data.username} SET search_path = '${local.f2-auth-db-namespace}';
    EOT
  }
}

resource "kubernetes_secret_v1" "f2-auth-db" {
  metadata {
    name      = "auth-db"
    namespace = var.namespace
    labels = {
      "cnpg.io/reload" = "true"
    }
  }

  data = {
    username = "[REDACTED]"
    password = random_password.f2-auth-db-password.result
    database = "auth"
  }

  type = "kubernetes.io/basic-auth"
}

resource "kubernetes_secret_v1" "f2-auth-jwt" {
  metadata {
    name      = "auth-jwt"
    namespace = var.namespace
  }

  data = {
    anonKey    = "[REDACTED]"
    secret     = "[REDACTED]"
    serviceKey = "[REDACTED]"
  }

  type = "Opaque"
}

resource "random_password" "f2-auth-db-password" {
  length           = 16
  special          = false
}

resource "kubernetes_deployment_v1" "f2-auth" {
  depends_on = [kubernetes_manifest.f2-auth-db]

  timeouts {
    create = "2m"
  }

  metadata {
    name = "f2auth"
    labels = {
      "f2.pub/app" = "auth-${var.environment}"
    }
    namespace = var.namespace
  }

  spec {
    replicas = 1

    selector {
      match_labels = {
        "f2.pub/app" = "auth-${var.environment}"
      }
    }

    template {
      metadata {
        labels = {
          "f2.pub/app" = "auth-${var.environment}"
        }
      }

      spec {
        image_pull_secrets { name = var.ghcr-pull-secret-name }

        init_container {
          name    = "init-db"
          image   = "postgres:17-alpine"
          command = ["psql", "-f", "/sql/script.sql"]

          env {
            name  = "PGHOST"
            value = "${kubernetes_manifest.f2-cluster.object.metadata.name}-rw"
          }

          env {
            name  = "PGPORT"
            value = "5432"
          }

          env {
            name  = "PGDATABASE"
            value = kubernetes_secret_v1.f2-auth-db.data.database
          }

          env {
            name  = "PGUSER"
            value = kubernetes_secret_v1.f2-auth-db.data.username
          }

          env {
            name  = "PGPASSWORD"
            value = kubernetes_secret_v1.f2-auth-db.data.password
          }

          volume_mount {
            name       = "sql-volume"
            mount_path = "/sql"
          }
        }

        volume {
          name = "sql-volume"

          config_map {
            name = kubernetes_config_map_v1.f2-auth-initdb.metadata[0].name
          }
        }

        container {
          image = "ghcr.io/siennathesane/auth:${var.goauth-version}"
          image_pull_policy = "Always"
          name  = "auth"

          resources {
            limits = {
              cpu    = "0.5"
              memory = "512Mi"
            }
            requests = {
              cpu    = "250m"
              memory = "50Mi"
            }
          }

          port {
            name           = "http"
            container_port = 9999
            protocol       = "TCP"
          }

          env {
            name  = "GOTRUE_DB_DRIVER"
            value = "postgres"
          }
          env {
            name  = "DB_NAMESPACE"
            value = "auth"
          }
          env {
            name  = "DATABASE_URL"
            value = "postgres://${kubernetes_secret_v1.f2-auth-db.data.username}:[REDACTED]@${ kubernetes_manifest.f2-cluster.object.metadata.name}-rw:5432/${kubernetes_secret_v1.f2-auth-db.data.database}"
          }
          env {
            name = "GOTRUE_JWT_SECRET"
            value_from {
              secret_key_ref {
                name = "auth-jwt"
                key  = "secret"
              }
            }
          }
          env {
            name  = "API_EXTERNAL_URL"
            value = "http://[REDACTED]"
          }
          env {
            name  = "GOTRUE_SITE_URL"
            value = "http://[REDACTED]"
          }
          env {
            name  = "GOTRUE_API_HOST"
            value = "0.0.0.0"
          }
          env {
            name  = "PORT"
            value = "9999"
          }
        }
      }
    }
  }
}
```

Closes #1729 
Closes #1848 
Closes #1983
Closes #2040

Signed-off-by: Sienna Satterwhite <[email protected]>
Co-authored-by: Chris Stockton <[email protected]>
@cangqiaoyuzhuo @cstockton
fix: fix the wrong error return value (#1950)
e2dfb5d 
## What kind of change does this PR introduce?

Bug fix

## What is the current behavior?


In fact, err is incorrect and should be terr.



## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.

Signed-off-by: cangqiaoyuzhuo <[email protected]>
Co-authored-by: Chris Stockton <[email protected]>
@cstockton
fix: check each type independently (#2290)
d9de0af 
Because type creation resides in the do block it is all or nothing. When
it's not a duplicate object that behavior is appropriate, but if it's
because one of the types already exists then you will fail the creation
of the other types.

For example:
* `aal_level` type already exists
* `factor_status` & `factor_type` are missing

Currently this would prevent `factor_status` & `factor_type` from ever
getting created.

Co-authored-by: Chris Stockton <[email protected]>
@cemalkilic
feat: allow amr claim to be array of strings or objects (#2274)
607da43 
## Summary

This PR loosens the validation for the `amr` (Authentication Method
Reference) claim in custom access token hooks to accept both array of
strings and array of objects, instead of only array of objects.

- **Test Coverage**: Added two new test cases:
- Modify amr to be array of strings - Verifies that `amr` as an array of
strings passes validation
- Modify amr to be array of objects - Verifies that `amr` as an array of
objects still works (backward compatibility)

## Motivation

This change provides more flexibility for custom access token hooks.
[RFC-8176
](https://www.rfc-editor.org/rfc/rfc8176.html#section-1)requires `amr`
to be array of strings.

## Testing

All tests pass, including:
- Existing custom access token tests
- New test for `amr` as array of strings
- New test for `amr` as array of objects

## Backward Compatibility

**Fully backward compatible** - The change only adds support for an
additional format. Existing hooks that return `amr` as an array of
objects will continue to work without any changes.
@fadymak
fix(indexworker): remove pg_trgm extension (#2301)
c553b10 
Removes dependency on the `pg_trgm` extension in the indexworker. This
simplifies the rollout process related to extension creation and
permissions at the cost of more restricted query patterns (i.e.: no
contains queries on name and email `%term%`).
@jnschaeffer
feat: Add Sb-Forwarded-For header and IP-based rate limiting (#2295)
e8f679b 
## What kind of change does this PR introduce?

This PR adds support for IP address forwarding using a new header,
`Sb-Forwarded-For`, optionally gated by
`GOTRUE_SECURITY_SB_FORWARDED_FOR_ENABLED`. When this feature is
enabled, both `utilities.GetIPAddress` and rate limiting will use the
first value of the `Sb-Forwarded-For` header as the IP address/rate
limiting key.

If the feature is disabled or the `Sb-Forwarded-For` header contains an
invalid value, Auth will fall back to existing behavior.

## What is the current behavior?

There are currently two paths along which users are likely to use IP
address information. The first is IP tracking (e.g., logging, MFA
challenge validation, and CAPTCHA challenge validation). The second is
rate limiting. Both of these follow slightly different logical paths,
relying on the `X-Forwarded-For` header explicitly in the former case
and a separate rate limiting key header in the latter.

The presence of these two paths results in some friction for users.
`X-Forwarded-For` can be (and frequently is) rewritten by proxies or
otherwise spoofed, and there is no guarantee that a rate limiting key in
the rate limit header is an IP address.

## What is the new behavior?

The API uses a new middleware, `sbff.Middleware`, that parses the
`Sb-Forwarded-For` header and inserts it into the request context if
`GOTRUE_SECURITY_SB_FORWARDED_FOR_ENABLED` is true. Consumers of the
`Sb-Forwarded-For` header can use `sbff.GetIPAddress` to retrieve the
parsed IP address.

`utilities.GetIPAddress` will prefer the result of `sbff.GetIPAddress`
as the end-user IP address if the feature is enabled and the
`Sb-Forwarded-For` header contains a value value. Similarly, Auth will
use the end user IP address as determined by `sbff.GetIPAddress` as the
rate limiting key under the same circumstances.

If the feature is not enabled or the `Sb-Forwarded-For` header is absent
or otherwise invalid, Auth will default to existing/legacy behavior.
@cstockton
fix: tighten email validation rules (#2304)
33bb372 
The goal here is to limit the conditions which resolver implementations
can affect the determinism of our DNS checks, without allowing transient
DNS failures to block signups:

* Reject single label email domains (`a@a`, `a@gmail`)
* Use absolute FQDN for DNS lookups to avoid implicit search behavior
* Preserves the RFC 5321 fallback, but narrows when it is called
* Add an allow list for major email providers to lower latency
* Reject mutated display name address that the mail parser might accept
* Add test coverage for some corner cases

Co-authored-by: Chris Stockton <[email protected]>
@cstockton
chore: update Go from v1.23.7 to v1.25.5 (#2303)
058836f 
Note: this PR depends on #2304

## Update to Go v1.25.5
As part of this upgrade several changes were needed to resolve go vet
failures around non-constant format strings:

- Updating all apierrors constructors to use const fmt strings with args
- Removing fmt.Sprintf usages that violate go vet fmt checks
- Refactoring internal error/message helpers to accept fmt + params

In addition stricter checks in the standard library for x509 certificate
creation required a change to a SAML test in internal/conf. Now I start
with a valid certificate and then set the serial number to an invalid
value. To do this I opted for a small refactor to PopulateFields to
return the cert object instead of copying the code within the test.

## Why update?
Aside from security related reasons and general best practices here are
some highlights im looking forward to!

* The [flight recorder](https://go.dev/blog/flight-recorder) could be
great for debugging performance bottlenecks. I personally am super
excited for this feature, I hope the new streamable tracing API will
enable a new class of interesting visual tools in the future.
* The [synctest](https://pkg.go.dev/testing/synctest) package means no
more DI for `now func() time.Time` :D
* Addition of [t.Context()](https://pkg.go.dev/testing#T.Context) and
[t.Cleanup(func())](https://pkg.go.dev/testing#T.Cleanup) will make
lower friction as we sever ties with our testing framework. There are
other neat API's like `T.Attr` and `T.Output`.
* The new [json/v2](https://pkg.go.dev/encoding/json/[email protected])
package offers more efficient memory usage and faster API's. When I've
ran profiling in the past JSON is a significant portion of our CPU time
so we should see some nice gains for this.
* The new
[jsontext](https://pkg.go.dev/encoding/json/[email protected]#example-package-StringReplace)
may come in handy for implementing JWT templates depending on how we
want to design it.
* And much more!

---------

Co-authored-by: Chris Stockton <[email protected]>
@cemalkilic
fix(oauth-server): allow custom URI schemes in client redirect URIs (#…
ea72f57 
…2298)

## Problem

Fixes #2285

The OAuth 2.0 Client Registration endpoin was incorrectly rejecting
custom URI schemes like `cursor://`, or `myapp://`, blocking native
application integrations.

## Root Cause
The validation logic in `validateRedirectURI()` was overly restrictive,
only allowing HTTPS or HTTP (localhost). This contradicted with **RFC
8252** (OAuth 2.0 for Native Apps) - recommends custom URI schemes

## Solution

Relaxed redirect URI validation

### New Validation Rules
  - **HTTPS** - always allowed
- **Custom URI schemes** - allowed for native apps (`cursor://`,
`myapp://`, `vscode://`, etc.)
- **HTTP** - only for localhost/loopback (`localhost`, `127.0.0.1`,
`::1`)
  - **Fragments** - still rejected per spec
@fadymak
chore: bump go version in docker (#2307)
3ffce52 
Bumps Go version to 1.25.5 in docker to match `go.mod`
@cemalkilic
fix: add issuer validation
b6e0e61
@cemalkilic cemalkilic requested a review from a team as a code owner January 9, 2026 07:39
@coveralls
Copy link

coveralls commented Jan 9, 2026

Pull Request Test Coverage Report for Build 20844809815

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 68.77%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/token_oidc.go 0 4 0.0%
Totals Coverage Status
Change from base Build 20599680500: -0.01%
Covered Lines: 14743
Relevant Lines: 21438
💛 - Coveralls

@hf hf changed the base branch from master to release/2.184.0 January 10, 2026 11:29
hf
hf requested changes Jan 12, 2026
View reviewed changes
Copy link
Contributor

@hf hf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has too many commits???

@hf
Copy link
Contributor

hf commented Jan 12, 2026

Ah nvm.

@hf hf closed this Jan 12, 2026
@hf hf deleted the cemal/fix-add-issuer-validator branch January 12, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hf hf hf requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@cemalkilic @coveralls @hf @jnschaeffer @cstockton @siennathesane @cangqiaoyuzhuo @fadymak