OpenConext / OpenConext/OpenConext-engineblock

Do not send SAML post form HTML from within controller

Open
#1,728 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

tech-debt
Dominant language
PHP
Stars
17
Forks
25
Avg merge
4d 1h
Merged PRs (30d)
1

Description

This issue is imported from pivotal - Originaly created at Oct 16, 2023 by Thijs Kinkhorst

In the current login process in Binding.php the parameters RelayState and return are constructed like this: In the controller, HTML input type=hidden form elements are constructed by concatenating strings. These are then passed this to the Twig template which renders them |raw. Obviously the HTML tags should be inside the template instead and no use of |raw should be necessary for this.

See https://github.com/OpenConext/OpenConext-engineblock/blob/d2c8583c14177cd6e24749e3319b8ec240e6bf14/library/EngineBlock/Corto/Module/Bindings.php#L671-L677

In order to successfully refactor this, functional tests for the presence of these parameters should be created, as they are currently not present so missing RelayState when it was set is not detected. (see #2056)

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read library/EngineBlock/Corto/Module/Bindings.php around lines 671-677 and the Twig template that receives the generated fields. Check the login flow for RelayState and return, then create functional tests for both parameters, including when RelayState is set. Done means the controller no longer constructs HTML or requires |raw, and the functional tests cover their presence.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authentication, security
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.