New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show custom rejection message to unauthorised users #997

Merged
merged 6 commits into from Nov 23, 2018

Conversation

Projects
None yet
3 participants
@dnmvisser
Contributor

dnmvisser commented Nov 22, 2018

Fixes #995.

@thijskh

Looks good! Some small comments added.
Would be nice to have a unit test, but since module completely lacks tests currently this is not a blocker.

Show resolved Hide resolved modules/authorize/lib/Auth/Process/Authorize.php Outdated
Show resolved Hide resolved modules/authorize/docs/authorize.md Outdated

dnmvisser added some commits Nov 23, 2018

@dnmvisser

This comment has been minimized.

Contributor

dnmvisser commented Nov 23, 2018

Should be all good now @thijskh

@thijskh thijskh merged commit 16a1422 into simplesamlphp:master Nov 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@thijskh thijskh added this to the 1.17 milestone Nov 23, 2018

@@ -2,7 +2,11 @@
{% block content %}
<h1>{{ '{authorize:Authorize:403_header}'|trans }}</h1>
{% if reject_msg[currentLanguage] is defined %}
<p>{{ reject_msg[currentLanguage]|raw }}</p>

This comment has been minimized.

@jaimeperez

jaimeperez Dec 12, 2018

Member

This isn't necessary. We have a function that obtains a localized string from an array containing the available translations, indexed by the language code. This is accessible from twig via the translateFromArray filter, so this code would be:

    {%- if reject_msg %}

    <p>{{ reject_msg|translateFromArray }}</p>
    {%- else %}

    <p>{% trans %}{authorize:Authorize:403_text}{% endtrans %}</p>
    {%- endif %}

A side note, also. Spacing is the hardest part of twig, IMHO. We should have good spacing / indentation in the twig templates, so that we can clearly see how the code flows, but the resulting HTML should also be properly indented. When we use twig tags, there can be space before and after them. If we have a newline right after the closure of a twig tag, that newline is removed from the output. The easiest way to control space in both the template and the resulting HTML is to use -. Adding it right after the begining of an opening tag ({%) tells twig to remove all the white space before, including newlines. Similarly, adding it before the end of a closing tag (-%}) removes all white space after it. So the simplest way for me to keep things working is to remember to add two newlines after a tag, and clear all white space before a tag that's standing on it's own line. E.g.:

    {%- if something %}

    <p>Something</p>
    {%- else %}

    <p>else</p>
    {%- endif %}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment