Skip to content
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

feat: added Refresh Token web interaction in userinfo.html #264

Merged
merged 28 commits into from
Jul 8, 2023

Conversation

rglauco
Copy link
Collaborator

@rglauco rglauco commented Jul 4, 2023

fixed #261
fixed specific version of pydantic

setup.py Outdated Show resolved Hide resolved
@@ -1 +1 @@
__version__ = "0.8.15"
__version__ = "0.8.16"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
__version__ = "0.8.16"
__version__ = "0.9.0"

what about this @rglauco ?
It seems that we're introducing several, even if "missing", features

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree on that

spid_cie_oidc/relying_party/views/rp_extend_session.py Outdated Show resolved Hide resolved
@peppelinux peppelinux changed the base branch from main to cjwt July 4, 2023 14:05
Comment on lines 48 to 66
<div class="col-12 p-4">
<h4>Attenzione:</h4>
<p>per abilitare le sessioni lunghe revocabili si deve valorizzare <em>OIDCFED_ACR_PROFILES</em> in settingslocal.py del Relying Party <em>OIDCFED_ACR_PROFILES=["https://www.spid.gov.it/SpidL2","https://www.spid.gov.it/SpidL1"]</em></p>
{% else %}
<div class="col-6 p-4">
<h4 class="text-left">ACCESS TOKEN</h4>
<p> L'access token <em>{{ rt_jti }}</em> <br />
scade tra <span id="at_time">{{ at_expiration }}</span> </p>
</div>
<div class="col-6 p-4">
<h4 class="text-left">REFRESH TOKEN <a href="{% url 'spid_cie_rp_extend_session' %}" class="btn btn-primary btn-xs btn-me" role="button">Prolunga Sessione</a></h4>

<p> Il Refresh Token <em>{{ rt_jti }}</em> <br />
scade tra <span id="rt_time">{{ rt_expiration }}</span> </p>
<div class="alert alert-warning" role="alert">
<p>NB: il numero di Refresh Token rilasciabili per una singola sessione &egrave; settato nella configurazione degli OP <em>OIDCFED_PROVIDER_MAX_REFRESH</em>. Superato questo numero il sistema restituisce errore HTTP/400</p>
</div>
</div>
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div class="col-12 p-4">
<h4>Attenzione:</h4>
<p>per abilitare le sessioni lunghe revocabili si deve valorizzare <em>OIDCFED_ACR_PROFILES</em> in settingslocal.py del Relying Party <em>OIDCFED_ACR_PROFILES=["https://www.spid.gov.it/SpidL2","https://www.spid.gov.it/SpidL1"]</em></p>
{% else %}
<div class="col-6 p-4">
<h4 class="text-left">ACCESS TOKEN</h4>
<p> L'access token <em>{{ rt_jti }}</em> <br />
scade tra <span id="at_time">{{ at_expiration }}</span> </p>
</div>
<div class="col-6 p-4">
<h4 class="text-left">REFRESH TOKEN <a href="{% url 'spid_cie_rp_extend_session' %}" class="btn btn-primary btn-xs btn-me" role="button">Prolunga Sessione</a></h4>
<p> Il Refresh Token <em>{{ rt_jti }}</em> <br />
scade tra <span id="rt_time">{{ rt_expiration }}</span> </p>
<div class="alert alert-warning" role="alert">
<p>NB: il numero di Refresh Token rilasciabili per una singola sessione &egrave; settato nella configurazione degli OP <em>OIDCFED_PROVIDER_MAX_REFRESH</em>. Superato questo numero il sistema restituisce errore HTTP/400</p>
</div>
</div>
{% endif %}
<div class="col-12 p-4">
<h4>{% trans "Attenzione" %}:</h4>
<p>{% trans 'Per abilitare le sessioni lunghe revocabili si deve valorizzare <em>OIDCFED_ACR_PROFILES</em> in settingslocal.py del Relying Party <em>OIDCFED_ACR_PROFILES=["https://www.spid.gov.it/SpidL2","https://www.spid.gov.it/SpidL1"]</em>' %}</p>
{% else %}
<div class="col-6 p-4">
<h4 class="text-left">ACCESS TOKEN</h4>
<p> L'access token <em>{{ rt_jti }}</em> <br />
scade tra <span id="at_time">{{ at_expiration }}</span> </p>
</div>
<div class="col-6 p-4">
<h4 class="text-left">REFRESH TOKEN <a href="{% url 'spid_cie_rp_extend_session' %}" class="btn btn-primary btn-xs btn-me" role="button">{% trans "Prolunga Sessione" %}</a></h4>
<p>{% trans "The" %} Refresh Token <em>{{ rt_jti }}</em> <br />
{% trans "scade tra" %} <span id="rt_time">{{ rt_expiration }}</span> </p>
<div class="alert alert-warning" role="alert">
<p>{% trans "NB: il numero di Refresh Token rilasciabili per una singola sessione &egrave; settato nella configurazione degli OP <em>OIDCFED_PROVIDER_MAX_REFRESH</em>. Superato questo numero il sistema restituisce errore HTTP/400" %}</p>
</div>
</div>
{% endif %}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the requirement to localize each text string and have by default english and not italian

@rglauco can you please apply my suggestion and then rework to english?

@@ -63,6 +63,18 @@ <h3 class="text-left">
{% endif %}
</a>
</li>
<li>
<a href="{% url 'spid_cie_rp_begin' %}?provider={{sub}}&profile=spid&scope=openid offline_access" id="provider-spid-refresh_token">
<span class="spid-sr-only">{{ attrs.organization_name }} SESSIONE A LUNGO TERMINE</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as well here, {% trans "" %} block is then required for every text string given to the users

Comment on lines 93 to 94
# code = "code",
# code_verifier = "code_verifier"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# code = "code",
# code_verifier = "code_verifier"


)
res = client.post(url, request)
self.assertTrue(res.status_code == 200)
refresh_token = verify_jws(res.json().get("refresh_token"), op_conf_priv_jwk)
self.assertEqual(refresh_token["sub"], RP_SUB)
self.assertEqual(refresh_token["aud"], RP_SUB)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aud could be the endpoint of the callback and not the entity_id/SUB

so this may be misleading

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed with RP_CLIENT_ID

Comment on lines 112 to 113
code="code",
code_verifier="code_verifier"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have removed this from the view but we keep these in the test

if removed in the refresh floe they should be removed from the test as well

self.authz = OidcSession.objects.filter(
auth_code=request.POST["code"],
revoked=False
).first()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else?

what happens if not auth code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged with the lower if/else

)

except Exception as e: # pragma: no cover
logger.error(f"Something went wrong: {e}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please extend this message, something went wrong "with what?"
please give a reference of the failed process/session/transaction

setup.py Outdated Show resolved Hide resolved
@peppelinux peppelinux merged commit 6912388 into italia:cjwt Jul 8, 2023
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad request response when send request for new access token using refresh token
2 participants