From 042c74f90c6eaf680ac7df6f0cca4e79800be0bb Mon Sep 17 00:00:00 2001 From: Slavi Pantaleev Date: Wed, 17 Jan 2024 14:45:19 +0200 Subject: [PATCH] Remove some useless oidc variables and /_synapse/oidc route handling After some checking, it seems like there's `/_synapse/client/oidc`, but no such thing as `/_synapse/oidc`. I'm not sure why we've been reverse-proxying these paths for so long (even in as far back as the `matrix-nginx-proxy` days), but it's time we put a stop to it. The OIDC docs have been simplified. There's no need to ask people to expose the useless `/_synapse/oidc` endpoint. OIDC requires `/_synapse/client/oidc` and `/_synapse/client` is exposed by default already. --- docs/configuring-playbook-synapse.md | 2 -- group_vars/matrix_servers | 1 - .../defaults/main.yml | 11 ------- .../tasks/validate_config.yml | 1 - .../templates/labels.j2 | 29 ------------------- roles/custom/matrix-synapse/defaults/main.yml | 14 --------- .../matrix-synapse/tasks/validate_config.yml | 1 - .../templates/synapse/labels.j2 | 29 ------------------- .../tasks/validate_config.yml | 22 ++++++++++++++ 9 files changed, 22 insertions(+), 88 deletions(-) diff --git a/docs/configuring-playbook-synapse.md b/docs/configuring-playbook-synapse.md index 81d945ef5..e9a3c3520 100644 --- a/docs/configuring-playbook-synapse.md +++ b/docs/configuring-playbook-synapse.md @@ -73,8 +73,6 @@ matrix_synapse_oidc_providers: backchannel_logout_enabled: true # Optional ``` -**NOTE**: if you inject the OIDC configuration using `matrix_synapse_configuration_extension_yaml` (instead of `matrix_synapse_oidc_enabled: true` + `matrix_synapse_oidc_providers` as explained above), then the OIDC routes (`/_synapse/oidc`) will not be publicly exposed automatically. In such a case, you'd need to expose them manually by toggling: `matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled: true`. - ## Customizing templates diff --git a/group_vars/matrix_servers b/group_vars/matrix_servers index 43127fe3b..d9456a3e8 100755 --- a/group_vars/matrix_servers +++ b/group_vars/matrix_servers @@ -4071,7 +4071,6 @@ matrix_synapse_reverse_proxy_companion_container_labels_traefik_tls_certResolver matrix_synapse_reverse_proxy_companion_container_labels_traefik_hostname: "{{ matrix_server_fqn_matrix }}" matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_enabled: "{{ matrix_synapse_container_labels_public_client_synapse_client_api_enabled }}" -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_enabled: "{{ matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled }}" matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_admin_api_enabled: "{{ matrix_synapse_container_labels_public_client_synapse_admin_api_enabled }}" matrix_synapse_reverse_proxy_companion_container_labels_public_federation_api_traefik_entrypoints: "{{ matrix_synapse_container_labels_public_federation_api_traefik_entrypoints }}" diff --git a/roles/custom/matrix-synapse-reverse-proxy-companion/defaults/main.yml b/roles/custom/matrix-synapse-reverse-proxy-companion/defaults/main.yml index 6aebc2449..5ab3b85a6 100644 --- a/roles/custom/matrix-synapse-reverse-proxy-companion/defaults/main.yml +++ b/roles/custom/matrix-synapse-reverse-proxy-companion/defaults/main.yml @@ -85,17 +85,6 @@ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_cl matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_traefik_tls: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_traefik_entrypoints != 'web' }}" matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_traefik_tls_certResolver: "{{ matrix_synapse_reverse_proxy_companion_container_labels_traefik_tls_certResolver }}" # noqa var-naming -# Controls whether labels will be added that expose the /_synapse/oidc paths -# Enable this if you need OpenID Connect authentication support. -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_enabled: false -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_hostname: "{{ matrix_synapse_reverse_proxy_companion_container_labels_traefik_hostname }}" -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_path_prefix: /_synapse/oidc -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_rule: "Host(`{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_hostname }}`) && PathPrefix(`{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_path_prefix }}`)" -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_priority: 0 -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_entrypoints: "{{ matrix_synapse_reverse_proxy_companion_container_labels_traefik_entrypoints }}" -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_tls: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_entrypoints != 'web' }}" -matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_tls_certResolver: "{{ matrix_synapse_reverse_proxy_companion_container_labels_traefik_tls_certResolver }}" # noqa var-naming - # Controls whether labels will be added that expose the /_synapse/admin paths # Following these recommendations (https://github.com/element-hq/synapse/blob/master/docs/reverse_proxy.md), by default, we don't. matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_admin_api_enabled: false diff --git a/roles/custom/matrix-synapse-reverse-proxy-companion/tasks/validate_config.yml b/roles/custom/matrix-synapse-reverse-proxy-companion/tasks/validate_config.yml index 5aac71e72..92d89b57a 100644 --- a/roles/custom/matrix-synapse-reverse-proxy-companion/tasks/validate_config.yml +++ b/roles/custom/matrix-synapse-reverse-proxy-companion/tasks/validate_config.yml @@ -11,7 +11,6 @@ - {'name': 'matrix_synapse_reverse_proxy_companion_container_labels_public_client_api_traefik_hostname', when: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_api_enabled }}"} - {'name': 'matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_traefik_hostname', when: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_client_api_enabled }}"} - - {'name': 'matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_hostname', when: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_enabled }}"} - {'name': 'matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_admin_api_traefik_hostname', when: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_admin_api_enabled }}"} - {'name': 'matrix_synapse_reverse_proxy_companion_container_labels_public_federation_api_traefik_hostname', when: "{{ matrix_synapse_reverse_proxy_companion_container_labels_public_federation_api_enabled }}"} diff --git a/roles/custom/matrix-synapse-reverse-proxy-companion/templates/labels.j2 b/roles/custom/matrix-synapse-reverse-proxy-companion/templates/labels.j2 index e2a580aca..44c7e7a82 100644 --- a/roles/custom/matrix-synapse-reverse-proxy-companion/templates/labels.j2 +++ b/roles/custom/matrix-synapse-reverse-proxy-companion/templates/labels.j2 @@ -91,35 +91,6 @@ traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synaps {% endif %} -{% if matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_enabled %} -############################################################ -# # -# Public Synapse OIDC API (/_synapse/oidc) # -# # -############################################################ - -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.rule={{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_rule }} - -{% if matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_priority | int > 0 %} -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.priority={{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_priority }} -{% endif %} - -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.service=matrix-synapse-reverse-proxy-companion-client-api -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.entrypoints={{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_entrypoints }} -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.tls={{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_tls | to_json }} - -{% if matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_tls %} -traefik.http.routers.matrix-synapse-reverse-proxy-companion-public-client-synapse-oidc-api.tls.certResolver={{ matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_api_traefik_tls_certResolver }} -{% endif %} - -############################################################ -# # -# /Public Synapse OIDC API (/_synapse/oidc) # -# # -############################################################ -{% endif %} - - {% if matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_admin_api_enabled %} ############################################################ # # diff --git a/roles/custom/matrix-synapse/defaults/main.yml b/roles/custom/matrix-synapse/defaults/main.yml index 2a8143ff3..008166f08 100644 --- a/roles/custom/matrix-synapse/defaults/main.yml +++ b/roles/custom/matrix-synapse/defaults/main.yml @@ -183,7 +183,6 @@ matrix_synapse_container_labels_traefik_hostname: '' # When set to false, variables like the following take no effect: # - `matrix_synapse_container_labels_public_client_api_enabled` # - `matrix_synapse_container_labels_public_client_synapse_client_api_enabled` -# - `matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled` # - `matrix_synapse_container_labels_public_client_synapse_admin_api_enabled` # - `matrix_synapse_container_labels_public_federation_api_enabled` # @@ -236,19 +235,6 @@ matrix_synapse_container_labels_public_client_synapse_client_api_traefik_entrypo matrix_synapse_container_labels_public_client_synapse_client_api_traefik_tls: "{{ matrix_synapse_container_labels_public_client_synapse_client_api_traefik_entrypoints != 'web' }}" matrix_synapse_container_labels_public_client_synapse_client_api_traefik_tls_certResolver: "{{ matrix_synapse_container_labels_traefik_tls_certResolver }}" # noqa var-naming -# Controls whether labels will be added that expose the /_synapse/oidc paths -# Enable this if you need OpenID Connect authentication support. -# Regardless of whether this is enabled, it may or may not take effect due to the value of other variables. -# See `matrix_synapse_container_labels_traefik_enabled` or `matrix_synapse_container_labels_matrix_related_labels_enabled` -matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled: "{{ matrix_synapse_oidc_enabled }}" -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_hostname: "{{ matrix_synapse_container_labels_traefik_hostname }}" -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_path_prefix: /_synapse/oidc -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_rule: "Host(`{{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_hostname }}`) && PathPrefix(`{{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_path_prefix }}`)" -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_priority: 0 -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_entrypoints: "{{ matrix_synapse_container_labels_traefik_entrypoints }}" -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_tls: "{{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_entrypoints != 'web' }}" -matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_tls_certResolver: "{{ matrix_synapse_container_labels_traefik_tls_certResolver }}" # noqa var-naming - # Controls whether labels will be added that expose the /_synapse/admin paths # Following these recommendations (https://github.com/element-hq/synapse/blob/master/docs/reverse_proxy.md), by default, we don't. # Regardless of whether this is enabled, it may or may not take effect due to the value of other variables. diff --git a/roles/custom/matrix-synapse/tasks/validate_config.yml b/roles/custom/matrix-synapse/tasks/validate_config.yml index cea1d4bee..ab389a432 100644 --- a/roles/custom/matrix-synapse/tasks/validate_config.yml +++ b/roles/custom/matrix-synapse/tasks/validate_config.yml @@ -24,7 +24,6 @@ - {'name': 'matrix_synapse_container_labels_internal_client_api_traefik_entrypoints', when: "{{ matrix_synapse_container_labels_internal_client_api_enabled }}"} - {'name': 'matrix_synapse_container_labels_public_client_synapse_client_api_traefik_hostname', when: "{{ matrix_synapse_container_labels_public_client_synapse_client_api_enabled }}"} - - {'name': 'matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_hostname', when: "{{ matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled }}"} - {'name': 'matrix_synapse_container_labels_public_client_synapse_admin_api_traefik_hostname', when: "{{ matrix_synapse_container_labels_public_client_synapse_admin_api_enabled }}"} - {'name': 'matrix_synapse_container_labels_public_federation_api_traefik_hostname', when: "{{ matrix_synapse_container_labels_public_federation_api_enabled }}"} diff --git a/roles/custom/matrix-synapse/templates/synapse/labels.j2 b/roles/custom/matrix-synapse/templates/synapse/labels.j2 index 9d68227ca..fd472b3fc 100644 --- a/roles/custom/matrix-synapse/templates/synapse/labels.j2 +++ b/roles/custom/matrix-synapse/templates/synapse/labels.j2 @@ -142,35 +142,6 @@ traefik.http.routers.matrix-synapse-public-client-synapse-client-api.tls.certRes {% endif %} -{% if matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled %} -############################################################ -# # -# Public Synapse OIDC API (/_synapse/oidc) # -# # -############################################################ - -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.rule={{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_rule }} - -{% if matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_priority | int > 0 %} -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.priority={{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_priority }} -{% endif %} - -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.service=matrix-synapse-client-api -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.entrypoints={{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_entrypoints }} -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.tls={{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_tls | to_json }} - -{% if matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_tls %} -traefik.http.routers.matrix-synapse-public-client-synapse-oidc-api.tls.certResolver={{ matrix_synapse_container_labels_public_client_synapse_oidc_api_traefik_tls_certResolver }} -{% endif %} - -############################################################ -# # -# /Public Synapse OIDC API (/_synapse/oidc) # -# # -############################################################ -{% endif %} - - {% if matrix_synapse_container_labels_public_client_synapse_admin_api_enabled %} ############################################################ # # diff --git a/roles/custom/matrix_playbook_migration/tasks/validate_config.yml b/roles/custom/matrix_playbook_migration/tasks/validate_config.yml index 982107080..bc428b386 100644 --- a/roles/custom/matrix_playbook_migration/tasks/validate_config.yml +++ b/roles/custom/matrix_playbook_migration/tasks/validate_config.yml @@ -339,3 +339,25 @@ You should remove all its variables (`matrix_ssl_*`) from your vars.yml file. We found usage of the following variables: {{ matrix_playbook_migration_ssl_migration_vars.keys() | join(', ') }} when: "matrix_playbook_migration_ssl_migration_vars | length > 0" + +- block: + - ansible.builtin.set_fact: + matrix_playbook_migration_matrix_synapse_container_labels_public_client_synapse_oidc_vars: |- + {{ vars | dict2items | selectattr('key', 'match', 'matrix_synapse_container_labels_public_client_synapse_oidc_*') | list | items2dict }} + + - name: (Deprecation) Catch and report matrix_ssl variables + ansible.builtin.fail: + msg: >- + We found usage of the following variables which are now removed: {{ matrix_playbook_migration_matrix_synapse_container_labels_public_client_synapse_oidc_vars.keys() | join(', ') }} + when: "matrix_playbook_migration_matrix_synapse_container_labels_public_client_synapse_oidc_vars | length > 0" + +- block: + - ansible.builtin.set_fact: + matrix_playbook_migration_matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_vars: |- + {{ vars | dict2items | selectattr('key', 'match', 'matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_*') | list | items2dict }} + + - name: (Deprecation) Catch and report matrix_ssl variables + ansible.builtin.fail: + msg: >- + We found usage of the following variables which are now removed: {{ matrix_playbook_migration_matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_vars.keys() | join(', ') }} + when: "matrix_playbook_migration_matrix_synapse_reverse_proxy_companion_container_labels_public_client_synapse_oidc_vars | length > 0"