Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ MESSENGER_TRANSPORT_DSN_FAILED='doctrine://default?queue_name=failed'
AZURE_AZ_OIDC_METADATA_URL=AZURE_AZ_APP_METADATA_URL
AZURE_AZ_OIDC_CLIENT_ID=AZURE_AZ_APP_CLIENT_ID
AZURE_AZ_OIDC_CLIENT_SECRET=AZURE_AZ_APP_CLIENT_SECRET
# Date the Azure client secret expires (any strtotime-parseable value)
AZURE_AZ_OIDC_CLIENT_SECRET_EXPIRES_AT=2027-01-31
AZURE_AZ_OIDC_REDIRECT_URI=AZURE_AZ_APP_REDIRECT_URI
AZURE_AZ_OIDC_ALLOW_HTTP=false
AZURE_AZ_OIDC_LEEWAY=10
Expand Down
5 changes: 5 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ APP_SECRET='$ecretf0rt3st'
SYMFONY_DEPRECATIONS_HELPER=999999
PANTHER_APP_ENV=panther
PANTHER_ERROR_SCREENSHOT_DIR=./var/error-screenshots

# The path of this URI is the only path the OIDC authenticator treats as a
# callback (openid-connect-bundle 6.0), so it has to be the app's own callback
# route for a callback to reach the authenticator at all.
AZURE_AZ_OIDC_REDIRECT_URI=https://itksites.example.org/openid-connect/generic
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`guzzlehttp/guzzle` 7.10.6 → 7.15.5, `guzzlehttp/psr7` 2.10.4 → 2.13.1
- Regenerated the API spec: `symfony/yaml` now writes sequence items on their
own line. No API changes
- [#92](https://github.com/itk-dev/devops_itksites/pull/92) Update openid-connect-bundle to 6.0
- Bump `itk-dev/openid-connect-bundle` to `^6.0`
- A failed OIDC callback now raises an error instead of redirecting to the
identity provider again, so an expired client secret can no longer put the
site in a login loop
- Only the provider's callback path is treated as a callback; the configured
`redirect_uri` covers this, no `callback_path` needed
- Set `client_secret_expires_at` for the `azure_az` provider from the new
`AZURE_AZ_OIDC_CLIENT_SECRET_EXPIRES_AT` variable, so the bundle warns
before the secret expires
- Render a failed login as a page saying so, instead of an unhandled
exception, in `OpenIdConnectFailureListener`
- Add an `oidc_client_secret` health check, so a client secret nearing its
expiry shows up in `/health/detail` instead of in a login loop
- [#83](https://github.com/itk-dev/devops_itksites/pull/83) 7523: Service agreements
- Add Project entity top-level Economics project.
- Add CodeOwner entity
Expand Down
15 changes: 11 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,21 @@ read `/health/detail` when it goes red:
curl --silent https://itksites.local.itkdev.dk/health/detail | jq
```

The checks cover the database, the RabbitMQ messenger transport and the
freshness of the most recent detection result. The last one catches an ingest
pipeline that has stopped while the application itself is still serving
requests.
The checks cover the database, the RabbitMQ messenger transport, the freshness
of the most recent detection result and the expiry of the OIDC client secret.
The freshness check catches an ingest pipeline that has stopped while the
application itself is still serving requests. The client secret check catches
the expiry that breaks every login at once.

`HEALTH_INGEST_MAX_AGE` sets how old the most recent detection result may be
before ingest is reported as degraded.

`AZURE_AZ_OIDC_CLIENT_SECRET_EXPIRES_AT` is where the client secret check reads
the date. It reports degraded only once that date has passed, so watch
`days_remaining` in the detail payload rather than waiting for it to go red. With
no date configured the check reports skipped, which means nothing is watching the
secret.

Results are cached for `HEALTH_CACHE_TTL` seconds so that polling does not turn
into load on the dependencies. The cache is the dedicated, filesystem-backed
`cache.health` pool in `config/packages/cache.yaml` – it has to keep working
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"doctrine/doctrine-migrations-bundle": "^4.0",
"doctrine/orm": "^3.0",
"easycorp/easyadmin-bundle": "^5.0",
"itk-dev/openid-connect-bundle": "^5.0",
"itk-dev/openid-connect-bundle": "^6.1",
"itk-dev/vault-bundle": "^1.0.0",
"nelmio/cors-bundle": "^2.2",
"ocramius/doctrine-batch-utils": "^2.8",
Expand Down
78 changes: 18 additions & 60 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions config/packages/itkdev_openid_connect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ itkdev_openid_connect:
metadata_url: '%env(string:AZURE_AZ_OIDC_METADATA_URL)%'
client_id: '%env(AZURE_AZ_OIDC_CLIENT_ID)%'
client_secret: '%env(AZURE_AZ_OIDC_CLIENT_SECRET)%'
# Date the client secret expires. Lets the bundle warn before it
# breaks every login, as it did on 2026-08-12.
client_secret_expires_at: '%env(string:AZURE_AZ_OIDC_CLIENT_SECRET_EXPIRES_AT)%'
# Specify redirect URI
redirect_uri: '%env(string:AZURE_AZ_OIDC_REDIRECT_URI)%'
allow_http: '%env(bool:AZURE_AZ_OIDC_ALLOW_HTTP)%'
Expand Down
5 changes: 4 additions & 1 deletion config/reference.php
Original file line number Diff line number Diff line change
Expand Up @@ -1768,12 +1768,15 @@
* metadata_url?: scalar|Param|null, // URL to OpenId Discovery Document
* client_id?: scalar|Param|null, // Client ID assigned by authorizer
* client_secret?: scalar|Param|null, // Client secret/password assigned by authorizer
* client_secret_expires_at?: scalar|Param|null, // Date the client secret expires, e.g. "2027-01-31". Anything strtotime() understands, and usually an environment variable. An expired secret breaks every login, so configuring this lets the bundle warn while there is still time to rotate. Will be required in 6.0. // Default: null
* client_secret_expires_at?: scalar|Param|null, // Optional. Date the client secret expires, e.g. "2027-01-31". Anything strtotime() understands, and usually an environment variable. Set it and the bundle warns before the secret expires; leave it unset and the provider reports "unknown" and is not monitored. Set it where the real secret lives — a date carried in a committed default is a date nobody maintains.
* leeway?: int|Param, // Leeway in seconds to account for clock skew between server and provider // Default: 10
* cache_duration?: int|Param, // Cache duration in seconds for the OIDC discovery document and JWKS (default: 86400 — 24 hours) // Default: 86400
* scopes?: Param|string|list<scalar|Param|null>,
* pkce?: bool|Param, // Send a PKCE challenge (RFC 7636, S256) with the authorization request // Default: true
* redirect_uri?: scalar|Param|null, // Redirect URI registered at identity provider
* redirect_route?: scalar|Param|null, // Redirect route registered at identity provider (must not be set if redirect_uri is set)
* redirect_route_parameters?: array<mixed>,
* callback_path?: scalar|Param|null, // Optional. The request path the callback arrives on, for a proxy that rewrites it without sending X-Forwarded-Prefix. Include any base path. Defaults to the path of redirect_uri, or of the generated redirect_route; a trusted X-Forwarded-Prefix or a subdirectory deployment is already accounted for without this.
* allow_http?: bool|Param, // Whether to allow http or not (default: false) // Default: false
* http_client_options?: array{ // Options forwarded to the underlying Guzzle HTTP client. league/oauth2-client only forwards: timeout, proxy, verify (verify is only consulted when proxy is set).
* timeout?: float|Param, // Total request timeout in seconds. Defaults to 30; set to 0 to wait indefinitely (Guzzle's own default). // Default: 30.0
Expand Down
82 changes: 82 additions & 0 deletions src/EventListener/OpenIdConnectFailureListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

declare(strict_types=1);

namespace App\EventListener;

use ItkDev\OpenIdConnectBundle\Exception\AuthenticationFailedException;
use ItkDev\OpenIdConnectBundle\Exception\ProviderErrorException;
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Twig\Environment;

/**
* Renders a failed OIDC login as a page rather than as an unhandled exception.
*
* A failed callback throws `AuthenticationFailedException`, which is deliberately
* not an `AuthenticationException`: the firewall does not catch it and does not
* send the browser back to Azure, which is what turned the expired client secret
* into a redirect loop. What escapes the firewall instead is an unhandled
* exception, so without this listener a failed login shows the default error page.
*
* `ProviderErrorException` is the subclass Azure sends when it refuses the request
* — a cancelled consent screen, an expired session there, a tenant policy. It
* carries the status the answer should have, and the page says the login was
* declined rather than that something went wrong.
*
* Only `AuthenticationFailedException` is handled. The bundle's other failures
* are configuration bugs — an unknown provider key, an unreachable
* `metadata_url` — and they keep the error page that says so.
*/
#[AsEventListener(
event: KernelEvents::EXCEPTION,
method: 'onKernelException',
// Below Symfony's ErrorListener::logKernelException (0), so the failure is
// still logged, and above its onKernelException (-128), whose rendering this
// replaces: setting a response stops propagation, so that one never runs.
priority: -64,
)]
readonly class OpenIdConnectFailureListener
{
public function __construct(
private Environment $twig,
private UrlGeneratorInterface $urlGenerator,
) {
}

public function onKernelException(ExceptionEvent $event): void
{
$exception = $event->getThrowable();

if (!$event->isMainRequest() || !$exception instanceof AuthenticationFailedException) {
return;
}

// Deliberately without the exception message: it carries the identity
// provider's own error text, which belongs in the log, not in a browser.
// `declined` is the one thing worth telling the user, and it is the error
// code rather than that text.
$content = $this->twig->render('error/openid_connect_failed.html.twig', [
'login_url' => $this->urlGenerator->generate('itkdev_openid_connect_login', ['providerKey' => 'azure_az']),
'declined' => $exception instanceof ProviderErrorException
&& ProviderErrorException::ACCESS_DENIED === $exception->getError(),
]);

// A refusal states its own status — 403 where the user or a policy said
// no, 503 where Azure reports its own trouble. Everything else is a 500:
// the likely cause is on this side of the login, and it should read as an
// error in the log and in monitoring.
$status = $exception instanceof HttpExceptionInterface
? $exception->getStatusCode()
: Response::HTTP_INTERNAL_SERVER_ERROR;

$response = new Response($content, $status);
$response->headers->set('Cache-Control', 'no-store, private');

$event->setResponse($response);
}
}
Loading
Loading