You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/07/28 01:09:10 UTC
[GitHub] [guacamole-client] jmuehlner opened a new pull request, #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
jmuehlner opened a new pull request, #748:
URL: https://github.com/apache/guacamole-client/pull/748
The previous approach didn't actually allow requests to be signed - this change modifies the way we're calling the SAML library to allow signatures to be appended to the request as query parameters.
Draft since more testing is needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #748:
URL: https://github.com/apache/guacamole-client/pull/748#discussion_r932609832
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/SAMLService.java:
##########
@@ -99,6 +117,11 @@ public URI createRequest() throws GuacamoleException {
+ "be generated due to an error in the URI syntax: "
+ e.getMessage());
}
+ catch (SettingsException e) {
+ throw new GuacamoleServerException("Error while attempting to sign "
+ + "request using provided private key / certificate: "
Review Comment:
Muuuuuuuch nicer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper merged pull request #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
Posted by GitBox <gi...@apache.org>.
mike-jumper merged PR #748:
URL: https://github.com/apache/guacamole-client/pull/748
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on pull request #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on PR #748:
URL: https://github.com/apache/guacamole-client/pull/748#issuecomment-1198470117
Ok, this is tested against a SAML provider that verifies signatures, and everything seems to be working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #748:
URL: https://github.com/apache/guacamole-client/pull/748#discussion_r932537869
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/SAMLService.java:
##########
@@ -98,6 +116,9 @@ public URI createRequest() throws GuacamoleException {
throw new GuacamoleServerException("SAML IdP redirect could not "
+ "be generated due to an error in the URI syntax: "
+ e.getMessage());
+ } catch (SettingsException e) {
Review Comment:
Please use the same style as already established here (don't cuddle the `catch`).
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/SAMLService.java:
##########
@@ -98,6 +116,9 @@ public URI createRequest() throws GuacamoleException {
throw new GuacamoleServerException("SAML IdP redirect could not "
+ "be generated due to an error in the URI syntax: "
+ e.getMessage());
+ } catch (SettingsException e) {
+ throw new GuacamoleServerException("SAML Settings errors: "
Review Comment:
Is there a more explanative contextual message we can provide here beyond "SAML Settings errors: ..."?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #748: GUACAMOLE-1372: Modify usage of SAML library to allow signed requests.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #748:
URL: https://github.com/apache/guacamole-client/pull/748#discussion_r932547974
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/SAMLService.java:
##########
@@ -98,6 +116,9 @@ public URI createRequest() throws GuacamoleException {
throw new GuacamoleServerException("SAML IdP redirect could not "
+ "be generated due to an error in the URI syntax: "
+ e.getMessage());
+ } catch (SettingsException e) {
+ throw new GuacamoleServerException("SAML Settings errors: "
Review Comment:
Sure, I dug into the source to find the situation where it would throw this error. Will update.
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/SAMLService.java:
##########
@@ -98,6 +116,9 @@ public URI createRequest() throws GuacamoleException {
throw new GuacamoleServerException("SAML IdP redirect could not "
+ "be generated due to an error in the URI syntax: "
+ e.getMessage());
+ } catch (SettingsException e) {
Review Comment:
Sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org