You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:53:41 UTC
[GitHub] [syncope] mmoayyed opened a new pull request #273: Switch to CAS 6.4 RC5
mmoayyed opened a new pull request #273:
URL: https://github.com/apache/syncope/pull/273
--
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
[GitHub] [syncope] ilgrosso commented on pull request #273: Switch to CAS 6.4 RC5
Posted by GitBox <gi...@apache.org>.
ilgrosso commented on pull request #273:
URL: https://github.com/apache/syncope/pull/273#issuecomment-860439625
@mmoayyed shouldn't pac4j be upgraded to 5.1.0 as well?
--
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
[GitHub] [syncope] ilgrosso commented on a change in pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r663656966
##########
File path: wa/starter/src/main/resources/log4j2.xml
##########
@@ -37,9 +37,14 @@ under the License.
<loggers>
- <asyncLogger name="org.apereo.cas" additivity="false" level="INFO">
+ <asyncLogger name="org.apereo.cas" additivity="false" level="DEBUG">
Review comment:
I think this `DEBUG` and the one following should be reverted to `INFO` - this file is supposed to be used by deployments.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed commented on a change in pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r663684568
##########
File path: wa/starter/src/main/resources/log4j2.xml
##########
@@ -37,9 +37,14 @@ under the License.
<loggers>
- <asyncLogger name="org.apereo.cas" additivity="false" level="INFO">
+ <asyncLogger name="org.apereo.cas" additivity="false" level="DEBUG">
Review comment:
OTOH, I think we should revert. You're right; I would prefer the system to be production ready OOTB.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed merged pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed merged pull request #273:
URL: https://github.com/apache/syncope/pull/273
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed commented on a change in pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r663683511
##########
File path: wa/starter/src/main/resources/log4j2.xml
##########
@@ -37,9 +37,14 @@ under the License.
<loggers>
- <asyncLogger name="org.apereo.cas" additivity="false" level="INFO">
+ <asyncLogger name="org.apereo.cas" additivity="false" level="DEBUG">
Review comment:
No objections, but wouldn't you say that having DEBUG level on is incredibly useful for a deployment as a starting point to troubleshoot and figure out what's going on? That is [the very first thing](https://apereo.github.io/cas/development/installation/Troubleshooting-Guide.html#review-logs) we do for deployment, or ask people to do when they trying to diagnose issues.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] ilgrosso commented on a change in pull request #273: Switch to CAS 6.4 RC5
Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r650693344
##########
File path: core/am/logic/src/main/java/org/apache/syncope/core/logic/wa/GoogleMfaAuthAccountLogic.java
##########
@@ -57,6 +58,26 @@ public void deleteFor(final String owner) {
});
}
+ @PreAuthorize("hasRole('" + IdRepoEntitlement.ANONYMOUS + "')")
+ public void deleteFor(final long id) {
Review comment:
Please rename this method as `delete()`
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Why removing disabled Spring repositories here?
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Isn't the information provided in the given link still valid?
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Isn't the information provided in the [given link](https://spring.io/blog/2020/10/29/notice-of-permissions-changes-to-repo-spring-io-fall-and-winter-2020) still valid?
There are dependencies that have such repos defined hence Syncope builds will report permission errors; we added such definitions to stop pulling from such repos.
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
There are dependencies that have such repos defined hence Syncope builds will report permission errors; we added such definitions to stop pulling from such repos.
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Isn't the information provided in the [given link](https://spring.io/blog/2020/10/29/notice-of-permissions-changes-to-repo-spring-io-fall-and-winter-2020) still valid?
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
When you remove such disabled repo definitions, you will notice that some 3rd party dependencies will be attempted to be downloaded from Spring repositories, failing with permission errors.
Anyway, let's keep the repo removal, I am going to build from your branch to see if I can still see such 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed commented on a change in pull request #273: Switch to CAS 6.4 RC5
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r650696389
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Why would you want to keep disabled repositories?
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
It is, but disabling a repository is effectively the same as a no-op. It's dead code, and certainly, the repositories are no longer needed and have been removed from upstream references. If you're telling maven, "don't use this repository for anything", then it would make sense to remove "unused" code.
##########
File path: pom.xml
##########
@@ -2045,40 +2005,6 @@ under the License.
<enabled>true</enabled>
</releases>
</repository>
-
- <!-- Disable some Spring repositories pulled in by dependencies - see
Review comment:
Could you point out what those dependencies are? Can we upgrade those? And I don't follow. If the repository is 100% disabled, how are those dependencies resolved?
--
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
[GitHub] [syncope] mmoayyed commented on pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on pull request #273:
URL: https://github.com/apache/syncope/pull/273#issuecomment-873060998
Update: I am working on fixing/diagnosing issues with OIDC/SAML2 tests. There are changes that need to be mad to handle dynamic issuers for OIDC, and almost none belong in the WA. The WA fixes are mostly sanitization of configuration, rather than any major changes, and I am hoping to finalize by early next week.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed merged pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed merged pull request #273:
URL: https://github.com/apache/syncope/pull/273
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed commented on pull request #273: Switch to CAS 6.4 RC5
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on pull request #273:
URL: https://github.com/apache/syncope/pull/273#issuecomment-860448515
Good point on pac4j; thanks for the note. I upgraded it.
--
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
[GitHub] [syncope] ilgrosso commented on a change in pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r663732471
##########
File path: pom.xml
##########
@@ -3002,16 +2939,16 @@ under the License.
</profiles>
<modules>
- <module>common</module>
- <module>core</module>
- <module>client</module>
+ <!-- <module>common</module> -->
Review comment:
Please revert this change
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] ilgrosso commented on a change in pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #273:
URL: https://github.com/apache/syncope/pull/273#discussion_r663732471
##########
File path: pom.xml
##########
@@ -3002,16 +2939,16 @@ under the License.
</profiles>
<modules>
- <module>common</module>
- <module>core</module>
- <module>client</module>
+ <!-- <module>common</module> -->
Review comment:
Please revert this change
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed edited a comment on pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed edited a comment on pull request #273:
URL: https://github.com/apache/syncope/pull/273#issuecomment-878369661
I would prefer to wait since the next RC would be out in a few days. I will probably do another merge with master right before the rc release to make sure everything runs ok and then we can merge this after the rc is out.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [syncope] mmoayyed commented on pull request #273: Switch to CAS 6.4 RC6
Posted by GitBox <gi...@apache.org>.
mmoayyed commented on pull request #273:
URL: https://github.com/apache/syncope/pull/273#issuecomment-878369661
I would prefer to wait since the next RC would be out on a few days. I will probably do another merge with master right before the rc release to make sure everything runs ok and then we can merge this after the rc is out.
--
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@syncope.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org