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.

Sign up for GitHub

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

Jump to bottom

Step Up #95

Merged
merged 10 commits into from
Jan 31, 2023
Merged

Step Up #95

merged 10 commits into from
Jan 31, 2023

Conversation

josephdecock
Copy link
Member

This sample shows how to set acr_values and/or max_age in an api response to trigger api step up. It ultimately uses the interaction response generator to require a new login based on the subject's claims and the acr_values and/or max_age parameters.

@josephdecock
Step Up
df3081c 
This sample shows how to set acr_values and max_age in an api response
to trigger api step up. It ultimately uses the interaction response
generator to require a new login based on the subject's claims and the
acr_values and/or max_age parameters.
This was linked to issues Dec 16, 2022
Sample for API step up #82
Closed
Samples for authZ request validator and the interaction response generator #67
Closed
@josephdecock
Copy link
Member Author

Note, I think we may want more samples for authz request validator specifically - I didn't end up using it at all in this one.

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 16, 2022
View reviewed changes
...er/v6/UserInteraction/StepUp/Client/wwwroot/lib/jquery-validation/dist/additional-methods.js

// Same as url, but TLD is optional
$.validator.addMethod( "url2", function( value, element ) {
return this.optional( element ) || /^(https?|ftp):\/\/(((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:)*@)?(((\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5]))|((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)*(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?)(:\d*)?)(\/((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)+(\/(([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)*)*)?)?(\?((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|[\uE000-\uF8FF]|\/|\?)*)?(#((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|\/|\?)*)?$/i.test( value );

Check failure

Code scanning / CodeQL

Inefficient regular expression

This part of the regular expression may cause exponential backtracking on strings starting with 'ftp://' and containing many repetitions of '0.0.'.
...er/v6/UserInteraction/StepUp/Client/wwwroot/lib/jquery-validation/dist/additional-methods.js

// Same as url, but TLD is optional
$.validator.addMethod( "url2", function( value, element ) {
return this.optional( element ) || /^(https?|ftp):\/\/(((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:)*@)?(((\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5]))|((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)*(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?)(:\d*)?)(\/((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)+(\/(([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)*)*)?)?(\?((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|[\uE000-\uF8FF]|\/|\?)*)?(#((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|\/|\?)*)?$/i.test( value );

Check failure

Code scanning / CodeQL

Inefficient regular expression

This part of the regular expression may cause exponential backtracking on strings starting with 'ftp://a' and containing many repetitions of '0.0'.
...erver/v6/UserInteraction/StepUp/Client/wwwroot/lib/jquery-validation/dist/jquery.validate.js
// https://gist.github.com/dperini/729294
// see also https://mathiasbynens.be/demo/url-regex
// modified to allow protocol-relative URLs
return this.optional( element ) || /^(?:(?:(?:https?|ftp):)?\/\/)(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})).?)(?::\d{2,5})?(?:[/?#]\S*)?$/i.test( value );

Check failure

Code scanning / CodeQL

Inefficient regular expression

This part of the regular expression may cause exponential backtracking on strings starting with '//0.' and containing many repetitions of '00.'.
IdentityServer/v6/UserInteraction/StepUp/IdentityServerHost/wwwroot/js/signin-redirect.js
@@ -0,0 +1 @@
window.location.href = document.querySelector("meta[http-equiv=refresh]").getAttribute("data-url");

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML

[DOM text](1) is reinterpreted as HTML without escaping meta-characters.
@brockallen brockallen mentioned this pull request Jan 10, 2023
Closed
@brockallen
Copy link
Member

Max age is already checked by the default AIRG: https://github.com/DuendeSoftware/IdentityServer/blob/main/src/IdentityServer/ResponseHandling/Default/AuthorizeInteractionResponseGenerator.cs#L208-L217

@brockallen
Copy link
Member

And FWIW, the IAuthorizationMiddlewareResultHandler is only used when the authorization policy runs as part of the UseAuthorization MW. That's why ditched its use in BFF, since people could use policies from any place. Not that it matters here, just more of a note.

@brockallen
Copy link
Member

So the last couple of ideas that might be nice for the sample:

  1. Detect the acr values and let the user know these are being asked for, even tho the user is already logged in

  2. Add a fake MFA page (with just a button even) to show how/where you'd link that in from the login page, and then the button would re-issue the current user session but now with a MFA claim as needed.

@brockallen brockallen mentioned this pull request Jan 19, 2023
Closed
@josephdecock
Copy link
Member Author

And FWIW, the IAuthorizationMiddlewareResultHandler is only used when the authorization policy runs as part of the UseAuthorization MW. That's why ditched its use in BFF, since people could use policies from any place. Not that it matters here, just more of a note.

Interesting ... I want to take a look at what the BFF does instead. What am I looking for?

@brockallen
Copy link
Member

Interesting ... I want to take a look at what the BFF does instead. What am I looking for?

I've forgotten, but perhaps here?

https://github.com/DuendeSoftware/BFF/blob/main/src/Duende.Bff/EndpointProcessing/BffAuthenticationService.cs#L47

@josephdecock
Copy link
Member Author

I've forgotten, but perhaps here?

https://github.com/DuendeSoftware/BFF/blob/main/src/Duende.Bff/EndpointProcessing/BffAuthenticationService.cs#L47

Thanks, that's what I was looking for!

@brockallen brockallen merged commit a74365e into DuendeSoftware:main Jan 31, 2023
@josephdecock josephdecock deleted the step-up branch February 2, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brockallen brockallen brockallen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sample for API step up Samples for authZ request validator and the interaction response generator
2 participants
@josephdecock @brockallen