Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6ddbcaec70 | ||
|
|
3ff5b7aa26 |
@@ -77,7 +77,12 @@ $config = Config::fromArray([
|
|||||||
$client = new AuthClient($config);
|
$client = new AuthClient($config);
|
||||||
|
|
||||||
// Step 1: redirect user to provider
|
// Step 1: redirect user to provider
|
||||||
$auth = $client->buildAuthorisationUrl();
|
$auth = $client->buildAuthorisationUrl([
|
||||||
|
'extra' => [
|
||||||
|
// Some providers require nonce for OIDC requests.
|
||||||
|
'nonce' => rtrim(strtr(base64_encode(random_bytes(32)), '+/', '-_'), '='),
|
||||||
|
],
|
||||||
|
]);
|
||||||
header('Location: ' . $auth['url']);
|
header('Location: ' . $auth['url']);
|
||||||
exit;
|
exit;
|
||||||
```
|
```
|
||||||
@@ -123,10 +128,13 @@ This wrapper now delegates to `BlackWall\Auth\AuthClient`.
|
|||||||
## Security notes
|
## Security notes
|
||||||
|
|
||||||
- Always validate OAuth `state` in callback handlers.
|
- Always validate OAuth `state` in callback handlers.
|
||||||
|
- For OIDC providers, include a per-request `nonce` in authorisation requests.
|
||||||
- Always use HTTPS in production.
|
- Always use HTTPS in production.
|
||||||
- Store refresh tokens securely.
|
- Store refresh tokens securely.
|
||||||
- Keep access tokens out of logs and browser-visible output.
|
- Keep access tokens out of logs and browser-visible output.
|
||||||
- Use short session lifetimes where possible.
|
- Use short session lifetimes where possible.
|
||||||
|
- `Config::fromArray()` enforces HTTPS URLs by default.
|
||||||
|
- For localhost-only development over HTTP, set `allowInsecureHttp => true`.
|
||||||
|
|
||||||
## Repository layout
|
## Repository layout
|
||||||
|
|
||||||
|
|||||||
@@ -25,7 +25,15 @@ $client = new AuthClient(Config::fromArray([
|
|||||||
]));
|
]));
|
||||||
|
|
||||||
if (!isset($_SESSION['user'])) {
|
if (!isset($_SESSION['user'])) {
|
||||||
$auth = $client->buildAuthorisationUrl();
|
$auth = $client->buildAuthorisationUrl([
|
||||||
|
'extra' => [
|
||||||
|
// Some providers require nonce for OIDC requests.
|
||||||
|
'nonce' => rtrim(strtr(base64_encode(random_bytes(32)), '+/', '-_'), '='),
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
// Prevent stale redirects from being reused from cache.
|
||||||
|
header('Cache-Control: no-store, no-cache, must-revalidate, max-age=0');
|
||||||
|
header('Pragma: no-cache');
|
||||||
header('Location: ' . $auth['url']);
|
header('Location: ' . $auth['url']);
|
||||||
exit;
|
exit;
|
||||||
}
|
}
|
||||||
@@ -54,6 +62,12 @@ $client = new AuthClient(Config::fromArray([
|
|||||||
'redirectUri' => 'https://your-app.example/callback.php',
|
'redirectUri' => 'https://your-app.example/callback.php',
|
||||||
]));
|
]));
|
||||||
|
|
||||||
|
if (isset($_GET['error'])) {
|
||||||
|
$description = isset($_GET['error_description']) ? (string) $_GET['error_description'] : 'Unknown provider error';
|
||||||
|
http_response_code(400);
|
||||||
|
exit('Authorization failed: ' . $description);
|
||||||
|
}
|
||||||
|
|
||||||
$result = $client->handleCallback($_GET);
|
$result = $client->handleCallback($_GET);
|
||||||
|
|
||||||
$_SESSION['user'] = [
|
$_SESSION['user'] = [
|
||||||
@@ -122,5 +136,8 @@ try {
|
|||||||
- Keep provider URLs in environment variables, not hard-coded.
|
- Keep provider URLs in environment variables, not hard-coded.
|
||||||
- Avoid printing tokens in production pages.
|
- Avoid printing tokens in production pages.
|
||||||
- Rotate client secrets for confidential clients.
|
- Rotate client secrets for confidential clients.
|
||||||
|
- Add a unique `nonce` to each OIDC authorisation request.
|
||||||
- Validate `state` on every callback request.
|
- Validate `state` on every callback request.
|
||||||
- Use secure session cookies (`Secure`, `HttpOnly`, `SameSite=Lax` or stricter).
|
- Use secure session cookies (`Secure`, `HttpOnly`, `SameSite=Lax` or stricter).
|
||||||
|
- HTTPS URLs are enforced by default in `Config::fromArray()`.
|
||||||
|
- For localhost-only HTTP testing, explicitly set `allowInsecureHttp => true`.
|
||||||
|
|||||||
+50
-6
@@ -13,7 +13,8 @@ final class Config
|
|||||||
public readonly string $redirectUri,
|
public readonly string $redirectUri,
|
||||||
public readonly ?string $userInfoUrl = null,
|
public readonly ?string $userInfoUrl = null,
|
||||||
public readonly ?string $clientSecret = null,
|
public readonly ?string $clientSecret = null,
|
||||||
public readonly string $defaultScope = 'openid profile email'
|
public readonly string $defaultScope = 'openid profile email',
|
||||||
|
public readonly bool $allowInsecureHttp = false
|
||||||
) {
|
) {
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -28,14 +29,57 @@ final class Config
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$allowInsecureHttp = isset($config['allowInsecureHttp']) && (bool) $config['allowInsecureHttp'];
|
||||||
|
$authorizeUrl = rtrim(trim((string) $config['authorizeUrl']), '/');
|
||||||
|
$tokenUrl = rtrim(trim((string) $config['tokenUrl']), '/');
|
||||||
|
$redirectUri = trim((string) $config['redirectUri']);
|
||||||
|
$userInfoUrl = isset($config['userInfoUrl']) ? rtrim(trim((string) $config['userInfoUrl']), '/') : null;
|
||||||
|
|
||||||
|
self::assertSecureUrl($authorizeUrl, 'authorizeUrl', $allowInsecureHttp);
|
||||||
|
self::assertSecureUrl($tokenUrl, 'tokenUrl', $allowInsecureHttp);
|
||||||
|
self::assertSecureUrl($redirectUri, 'redirectUri', $allowInsecureHttp);
|
||||||
|
if ($userInfoUrl !== null && $userInfoUrl !== '') {
|
||||||
|
self::assertSecureUrl($userInfoUrl, 'userInfoUrl', $allowInsecureHttp);
|
||||||
|
}
|
||||||
|
|
||||||
return new self(
|
return new self(
|
||||||
clientId: $config['clientId'],
|
clientId: $config['clientId'],
|
||||||
authorizeUrl: rtrim($config['authorizeUrl'], '/'),
|
authorizeUrl: $authorizeUrl,
|
||||||
tokenUrl: rtrim($config['tokenUrl'], '/'),
|
tokenUrl: $tokenUrl,
|
||||||
redirectUri: $config['redirectUri'],
|
redirectUri: $redirectUri,
|
||||||
userInfoUrl: isset($config['userInfoUrl']) ? rtrim((string) $config['userInfoUrl'], '/') : null,
|
userInfoUrl: $userInfoUrl,
|
||||||
clientSecret: isset($config['clientSecret']) ? (string) $config['clientSecret'] : null,
|
clientSecret: isset($config['clientSecret']) ? (string) $config['clientSecret'] : null,
|
||||||
defaultScope: isset($config['scope']) ? (string) $config['scope'] : 'openid profile email'
|
defaultScope: isset($config['scope']) ? (string) $config['scope'] : 'openid profile email',
|
||||||
|
allowInsecureHttp: $allowInsecureHttp
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static function assertSecureUrl(string $url, string $field, bool $allowInsecureHttp): void
|
||||||
|
{
|
||||||
|
$parts = parse_url($url);
|
||||||
|
if ($parts === false || !isset($parts['scheme'])) {
|
||||||
|
throw new \InvalidArgumentException("{$field} must be a valid absolute URL");
|
||||||
|
}
|
||||||
|
|
||||||
|
$scheme = strtolower((string) $parts['scheme']);
|
||||||
|
if (!in_array($scheme, ['https', 'http'], true)) {
|
||||||
|
throw new \InvalidArgumentException("{$field} must use http or https");
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($scheme === 'https') {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($allowInsecureHttp && self::isLocalHost((string) ($parts['host'] ?? ''))) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new \InvalidArgumentException("{$field} must use https (set allowInsecureHttp=true for localhost development)");
|
||||||
|
}
|
||||||
|
|
||||||
|
private static function isLocalHost(string $host): bool
|
||||||
|
{
|
||||||
|
$normalized = strtolower(trim($host));
|
||||||
|
return in_array($normalized, ['localhost', '127.0.0.1', '::1'], true);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,15 +8,19 @@ use BlackWall\Auth\Exception\TransportException;
|
|||||||
|
|
||||||
final class CurlHttpClient implements HttpClientInterface
|
final class CurlHttpClient implements HttpClientInterface
|
||||||
{
|
{
|
||||||
public function __construct(private readonly int $timeoutSeconds = 20)
|
public function __construct(
|
||||||
|
private readonly int $timeoutSeconds = 20,
|
||||||
|
private readonly int $connectTimeoutSeconds = 10
|
||||||
|
)
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
public function postForm(string $url, array $fields, array $headers = []): array
|
public function postForm(string $url, array $fields, array $headers = []): array
|
||||||
{
|
{
|
||||||
|
$this->assertValidUrl($url);
|
||||||
$ch = curl_init($url);
|
$ch = curl_init($url);
|
||||||
if ($ch === false) {
|
if ($ch === false) {
|
||||||
throw new TransportException('Unable to initialise cURL for POST request');
|
throw new TransportException('Unable to initialise cURL for POST request', 'curl_init_failed');
|
||||||
}
|
}
|
||||||
|
|
||||||
$headerLines = $this->normaliseHeaders(array_merge([
|
$headerLines = $this->normaliseHeaders(array_merge([
|
||||||
@@ -29,16 +33,31 @@ final class CurlHttpClient implements HttpClientInterface
|
|||||||
CURLOPT_POSTFIELDS => http_build_query($fields, '', '&', PHP_QUERY_RFC3986),
|
CURLOPT_POSTFIELDS => http_build_query($fields, '', '&', PHP_QUERY_RFC3986),
|
||||||
CURLOPT_HTTPHEADER => $headerLines,
|
CURLOPT_HTTPHEADER => $headerLines,
|
||||||
CURLOPT_RETURNTRANSFER => true,
|
CURLOPT_RETURNTRANSFER => true,
|
||||||
|
CURLOPT_CONNECTTIMEOUT => $this->connectTimeoutSeconds,
|
||||||
CURLOPT_TIMEOUT => $this->timeoutSeconds,
|
CURLOPT_TIMEOUT => $this->timeoutSeconds,
|
||||||
|
CURLOPT_SSL_VERIFYPEER => true,
|
||||||
|
CURLOPT_SSL_VERIFYHOST => 2,
|
||||||
|
CURLOPT_FOLLOWLOCATION => false,
|
||||||
|
CURLOPT_MAXREDIRS => 0,
|
||||||
]);
|
]);
|
||||||
|
if (defined('CURLOPT_PROTOCOLS')) {
|
||||||
|
curl_setopt($ch, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);
|
||||||
|
}
|
||||||
|
if (defined('CURLOPT_REDIR_PROTOCOLS')) {
|
||||||
|
curl_setopt($ch, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);
|
||||||
|
}
|
||||||
|
|
||||||
$body = curl_exec($ch);
|
$body = curl_exec($ch);
|
||||||
$status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
|
$status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
|
||||||
|
$errno = curl_errno($ch);
|
||||||
$error = curl_error($ch);
|
$error = curl_error($ch);
|
||||||
curl_close($ch);
|
curl_close($ch);
|
||||||
|
|
||||||
if ($body === false) {
|
if ($body === false) {
|
||||||
throw new TransportException($error !== '' ? $error : 'Unknown cURL POST error');
|
throw new TransportException(
|
||||||
|
$error !== '' ? $error : 'Unknown cURL POST error',
|
||||||
|
$errno > 0 ? ('curl_errno_' . $errno) : 'curl_post_failed'
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return ['status' => $status, 'body' => $body];
|
return ['status' => $status, 'body' => $body];
|
||||||
@@ -46,9 +65,10 @@ final class CurlHttpClient implements HttpClientInterface
|
|||||||
|
|
||||||
public function get(string $url, array $headers = []): array
|
public function get(string $url, array $headers = []): array
|
||||||
{
|
{
|
||||||
|
$this->assertValidUrl($url);
|
||||||
$ch = curl_init($url);
|
$ch = curl_init($url);
|
||||||
if ($ch === false) {
|
if ($ch === false) {
|
||||||
throw new TransportException('Unable to initialise cURL for GET request');
|
throw new TransportException('Unable to initialise cURL for GET request', 'curl_init_failed');
|
||||||
}
|
}
|
||||||
|
|
||||||
$headerLines = $this->normaliseHeaders(array_merge([
|
$headerLines = $this->normaliseHeaders(array_merge([
|
||||||
@@ -58,16 +78,31 @@ final class CurlHttpClient implements HttpClientInterface
|
|||||||
curl_setopt_array($ch, [
|
curl_setopt_array($ch, [
|
||||||
CURLOPT_HTTPHEADER => $headerLines,
|
CURLOPT_HTTPHEADER => $headerLines,
|
||||||
CURLOPT_RETURNTRANSFER => true,
|
CURLOPT_RETURNTRANSFER => true,
|
||||||
|
CURLOPT_CONNECTTIMEOUT => $this->connectTimeoutSeconds,
|
||||||
CURLOPT_TIMEOUT => $this->timeoutSeconds,
|
CURLOPT_TIMEOUT => $this->timeoutSeconds,
|
||||||
|
CURLOPT_SSL_VERIFYPEER => true,
|
||||||
|
CURLOPT_SSL_VERIFYHOST => 2,
|
||||||
|
CURLOPT_FOLLOWLOCATION => false,
|
||||||
|
CURLOPT_MAXREDIRS => 0,
|
||||||
]);
|
]);
|
||||||
|
if (defined('CURLOPT_PROTOCOLS')) {
|
||||||
|
curl_setopt($ch, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);
|
||||||
|
}
|
||||||
|
if (defined('CURLOPT_REDIR_PROTOCOLS')) {
|
||||||
|
curl_setopt($ch, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS);
|
||||||
|
}
|
||||||
|
|
||||||
$body = curl_exec($ch);
|
$body = curl_exec($ch);
|
||||||
$status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
|
$status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
|
||||||
|
$errno = curl_errno($ch);
|
||||||
$error = curl_error($ch);
|
$error = curl_error($ch);
|
||||||
curl_close($ch);
|
curl_close($ch);
|
||||||
|
|
||||||
if ($body === false) {
|
if ($body === false) {
|
||||||
throw new TransportException($error !== '' ? $error : 'Unknown cURL GET error');
|
throw new TransportException(
|
||||||
|
$error !== '' ? $error : 'Unknown cURL GET error',
|
||||||
|
$errno > 0 ? ('curl_errno_' . $errno) : 'curl_get_failed'
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return ['status' => $status, 'body' => $body];
|
return ['status' => $status, 'body' => $body];
|
||||||
@@ -81,9 +116,30 @@ final class CurlHttpClient implements HttpClientInterface
|
|||||||
{
|
{
|
||||||
$result = [];
|
$result = [];
|
||||||
foreach ($headers as $name => $value) {
|
foreach ($headers as $name => $value) {
|
||||||
|
if (
|
||||||
|
str_contains((string) $name, "\r")
|
||||||
|
|| str_contains((string) $name, "\n")
|
||||||
|
|| str_contains((string) $value, "\r")
|
||||||
|
|| str_contains((string) $value, "\n")
|
||||||
|
) {
|
||||||
|
throw new TransportException('Header names/values must not contain CR/LF characters', 'invalid_header');
|
||||||
|
}
|
||||||
$result[] = sprintf('%s: %s', $name, $value);
|
$result[] = sprintf('%s: %s', $name, $value);
|
||||||
}
|
}
|
||||||
|
|
||||||
return $result;
|
return $result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function assertValidUrl(string $url): void
|
||||||
|
{
|
||||||
|
$parts = parse_url($url);
|
||||||
|
if ($parts === false || !isset($parts['scheme'])) {
|
||||||
|
throw new TransportException('Request URL must be an absolute URL', 'invalid_url');
|
||||||
|
}
|
||||||
|
|
||||||
|
$scheme = strtolower((string) $parts['scheme']);
|
||||||
|
if (!in_array($scheme, ['http', 'https'], true)) {
|
||||||
|
throw new TransportException('Only http/https URLs are supported', 'invalid_url_scheme');
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,39 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace BlackWall\Auth\Tests;
|
||||||
|
|
||||||
|
use BlackWall\Auth\Config;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
|
||||||
|
final class ConfigSecurityTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testRejectsInsecureHttpForNonLocalhostByDefault(): void
|
||||||
|
{
|
||||||
|
$this->expectException(\InvalidArgumentException::class);
|
||||||
|
Config::fromArray([
|
||||||
|
'clientId' => 'client-123',
|
||||||
|
'authorizeUrl' => 'http://provider.example/oauth/authorize',
|
||||||
|
'tokenUrl' => 'https://provider.example/oauth/token',
|
||||||
|
'userInfoUrl' => 'https://provider.example/oauth/userinfo',
|
||||||
|
'redirectUri' => 'https://app.example/callback.php',
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testAllowsInsecureHttpForLocalhostWhenExplicitlyEnabled(): void
|
||||||
|
{
|
||||||
|
$config = Config::fromArray([
|
||||||
|
'clientId' => 'client-123',
|
||||||
|
'authorizeUrl' => 'http://localhost/oauth/authorize',
|
||||||
|
'tokenUrl' => 'http://127.0.0.1/oauth/token',
|
||||||
|
'userInfoUrl' => 'http://localhost/oauth/userinfo',
|
||||||
|
'redirectUri' => 'http://localhost/callback.php',
|
||||||
|
'allowInsecureHttp' => true,
|
||||||
|
]);
|
||||||
|
|
||||||
|
self::assertTrue($config->allowInsecureHttp);
|
||||||
|
self::assertSame('http://localhost/oauth/authorize', $config->authorizeUrl);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@@ -0,0 +1,31 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace BlackWall\Auth\Tests;
|
||||||
|
|
||||||
|
use BlackWall\Auth\Exception\TransportException;
|
||||||
|
use BlackWall\Auth\Http\CurlHttpClient;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
|
||||||
|
final class CurlHttpClientSecurityTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testRejectsUnsupportedUrlScheme(): void
|
||||||
|
{
|
||||||
|
$client = new CurlHttpClient();
|
||||||
|
|
||||||
|
$this->expectException(TransportException::class);
|
||||||
|
$client->get('file:///etc/passwd');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRejectsHeaderInjectionAttempt(): void
|
||||||
|
{
|
||||||
|
$client = new CurlHttpClient();
|
||||||
|
|
||||||
|
$this->expectException(TransportException::class);
|
||||||
|
$client->get('https://example.com', [
|
||||||
|
'X-Test' => "ok\r\nInjected: yes",
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Reference in New Issue
Block a user