Redirect URI should be able to contain plus (+) character

[dominikglandorf]

Describe the bug

Due to https://tools.ietf.org/html/rfc3986#section-3.1 the + character is allowed in URL schemes. So web+application://callback should be a valid callback URL. Using this for the OAuth2 authorization code flow results in:

Reproducing the bug

Steps to reproduce the behavior:

1. Create a client in Hydra, containing a plus in the redirect_uris:

{
  client_id: 'application',
  client_name: 'App',
  redirect_uris: [ 'web+application://callback' ],
  grant_types: [ 'authorization_code', 'refresh_token' ],
  response_types: [ 'code', 'token', 'id_token' ],
  scope: 'openid',
  audience: [],
  owner: '',
  policy_uri: '',
  allowed_cors_origins: [],
  tos_uri: '',
  client_uri: '',
  logo_uri: '',
  contacts: [],
  client_secret_expires_at: 0,
  subject_type: 'pairwise',
  jwks: {},
  token_endpoint_auth_method: 'client_secret_basic',
  userinfo_signed_response_alg: 'none',
  created_at: '2020-09-14T09:33:58Z',
  updated_at: '2020-09-14T09:33:58Z',
  metadata: {}
}

2. Call `http://localhost:9000/oauth2/auth?client_id=application&redirect_uri=web+application://callback&response_type=code&state=3d1pwr0z&scopes=openid

Server logs

time=2020-09-14T09:42:37Z level=error msg=An error occurred audience=application error=map[message:invalid_request reason:The "redirect_uri" parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls. status:Bad Request status_code:400] http_request=map[headers:map[accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 accept-encoding:gzip, deflate accept-language:de-de user-agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.2 Safari/605.1.15] host:localhost:9000 method:GET path:/oauth2/auth query:client_id=application&redirect_uri=web+application://callback&response_type=code&state=3d1pwr0z&scopes=openid remote:172.22.0.1:59436 scheme:http] service_name= service_version=

Server configuration

Dockerfile

version: '3'

services:

  hydra-postgres:
    image: postgres:9.6
    environment:
      - POSTGRES_USER
      - POSTGRES_PASSWORD
      - POSTGRES_DB
    ports:
      - 5432:5432
    volumes:
      - data-hydra:/var/lib/postgresql/data
    restart: on-failure

  hydra-migrate-database:
    image: oryd/hydra:${HYDRA_VERSION}
    command: ["migrate", "sql", "--yes", "$DSN"]

  hydra-host:
    image: oryd/hydra:${HYDRA_VERSION}
    ports:
      - ${PUBLIC_PORT}:4444
      - ${ADMIN_PORT}:4445
    environment:
      - SECRETS_SYSTEM
      - DSN
      - URLS_SELF_ISSUER
      - URLS_LOGIN=${SC_FRONTEND}/oauth2/login
      - URLS_CONSENT=${SC_FRONTEND}/oauth2/consent
      - TLS_ALLOW_TERMINATION_FROM
      - OIDC_SUBJECT_IDENTIFIERS_ENABLED=public,pairwise
      - OIDC_SUBJECT_IDENTIFIERS_PAIRWISE_SALT
      - LOG_LEAK_SENSITIVE_VALUES
      - OAUTH2_EXPOSE_INTERNAL_ERRORS
    depends_on:
      - hydra-migrate-database
    restart: on-failure
    command: ["serve", "all", "${SERVE_PARAMS}"]
    deploy:
      replicas: 1
      restart_policy:
        condition: any

volumes:
  data-hydra:

.env

HYDRA_VERSION=v1
PUBLIC_PORT=9000
ADMIN_PORT=9001
SECRETS_SYSTEM=supersecurityperfavore
DSN=postgres://hydra:secret@hydra-postgres:5432/hydra?sslmode=disable
TLS_ALLOW_TERMINATION_FROM=172.18.0.0/24
OIDC_SUBJECT_IDENTIFIERS_PAIRWISE_SALT=atleast8
URLS_SELF_ISSUER=http://localhost:9000
SC_FRONTEND=http://localhost:3100
POSTGRES_USER=hydra
POSTGRES_PASSWORD=secret
POSTGRES_DB=hydra
SERVE_PARAMS=--dangerous-force-http
LOG_LEAK_SENSITIVE_VALUES=true
OAUTH2_EXPOSE_INTERNAL_ERRORS=true

Expected behavior

The usual /oauth2/auth response without error.

Environment

• Version: v1.7.4
• Environment: Windows 10/Ubuntu, Docker

Additional context

Discussion in Ory Community: https://community.ory.sh/t/redirect-url-containing-plus-character/2158

[aeneasr]

Thank you for the report! Any contributions towards this would be appreciated. The fix would need to be done in ORY Fosite.

[mitar]

This is a duplicate of ory/fosite#464.

[mitar]

This has been fixed in fosite.

[janrenz]

@dominikglandorf I think this can be closed as a duplicate or fixed