You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2020/02/11 01:51:47 UTC
[GitHub] [knox] moresandeep opened a new pull request #262: KNOX-2231 - Fix
KnoxSSO OIDC integration
moresandeep opened a new pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262
## What changes were proposed in this pull request?
Fix broken OIDC KnoxSSO integration
## How was this patch tested?
This patch was locally tested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] moresandeep commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377434430
##########
File path: pom.xml
##########
@@ -2068,12 +2068,8 @@
<artifactId>pac4j-oidc</artifactId>
<version>${pac4j.version}</version>
<exclusions>
- <exclusion>
- <groupId>com.sun.mail</groupId>
- <artifactId>javax.mail</artifactId>
- </exclusion>
- <exclusion>
- <groupId>org.mockito</groupId>
+ <exclusion>
Review comment:
Turns out git UI does not handle spaces and tabs well. My IDE inserted tabs which messed up git UI. I'll change it to spaces.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] moresandeep commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377433806
##########
File path: gateway-test/pom.xml
##########
@@ -172,6 +172,12 @@
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
+ <exclusions>
Review comment:
That can be done but do we want to exclude this dependency for all the projects ? it should be OK here given that this is for test, it should not matter since we do check for unit tests before builds. Let me try that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] moresandeep merged pull request #262: KNOX-2231 - Fix
KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
moresandeep merged pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377432611
##########
File path: gateway-test/pom.xml
##########
@@ -172,6 +172,12 @@
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
+ <exclusions>
Review comment:
Exclude it at the top level pom then? Where rest-assured is defined?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on issue #262: KNOX-2231 - Fix KnoxSSO
OIDC integration
Posted by GitBox <gi...@apache.org>.
risdenk commented on issue #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#issuecomment-584720359
Looks good.
Future note to myself - it might make sense to avoid excluding `javax.activation` and instead include a specific version in `pom.xml`. I don't think it matters here, but just something to keep in mind.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] moresandeep commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377431910
##########
File path: gateway-test/pom.xml
##########
@@ -172,6 +172,12 @@
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
+ <exclusions>
Review comment:
javax.mail has a different version of javax.activation causing builds to breaks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377429842
##########
File path: pom.xml
##########
@@ -2068,12 +2068,8 @@
<artifactId>pac4j-oidc</artifactId>
<version>${pac4j.version}</version>
<exclusions>
- <exclusion>
- <groupId>com.sun.mail</groupId>
- <artifactId>javax.mail</artifactId>
- </exclusion>
- <exclusion>
- <groupId>org.mockito</groupId>
+ <exclusion>
Review comment:
nit: spacing is off
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #262:
KNOX-2231 - Fix KnoxSSO OIDC integration
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #262: KNOX-2231 - Fix KnoxSSO OIDC integration
URL: https://github.com/apache/knox/pull/262#discussion_r377429885
##########
File path: gateway-test/pom.xml
##########
@@ -172,6 +172,12 @@
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
+ <exclusions>
Review comment:
Why do we need this? This looks like a test only exclusion?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services