You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/01/02 04:16:17 UTC

[GitHub] [fineract] vidakovic opened a new pull request #2012: Simplify build for different auth strategies

vidakovic opened a new pull request #2012:
URL: https://github.com/apache/fineract/pull/2012


   ## Description
   
   The current process of selecting the authentication strategy for Fineract is unnecessarily complicated. Spring profiles are not the best way to enable/disable these features. By using "@ConditionalOnProperty" we only need one build and can enable the required authentication scheme with environment variables. This makes maintenance of the application.properties file also easier (just one instead of 4 different variants), and in turn makes the build script simpler.
   


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#issuecomment-1003663320


   @ptuomola just FYI: please approve and merge #2009 first


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on a change in pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on a change in pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#discussion_r777189120



##########
File path: oauth2-tests/build.gradle
##########
@@ -45,7 +45,7 @@ cargo {
         }
         startStopTimeout = 240000
         containerProperties {
-            property 'cargo.start.jvmargs', '-Dspring.profiles.active=oauth'
+            property 'cargo.start.jvmargs', '-Dfineract.security.basicauth.enabled=false -Dfineract.security.oauth.enabled=true -Dfineract.security.2fa.enabled=false'

Review comment:
       Correct, exactly the same behavior as before, just with properties instead of profiles.




-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#issuecomment-1003749079


   @ptuomola Hi Petri... finally... I think this should run through now properly... added a couple of last minute fixes to make the unit tests run again properly, do some minor cleanup (e. g. `TwoFactorUtil` not used anymore). I've also rebased with the latest in `develop`. As soon as this one is in I'll start the migration to Testcontainers. Would be great if you could approve and merge.


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on a change in pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on a change in pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#discussion_r777189236



##########
File path: .github/workflows/build.yml
##########
@@ -47,5 +56,24 @@ jobs:
         run: |
             sudo apt-get update
             sudo apt-get install ghostscript -y
-      - name: Build & Test
+
+      - name: Basic Auth Build & Test
+        env:
+          - FINERACT_SECURITY_BASICAUTH_ENABLED: true
+          - FINERACT_SECURITY_OAUTH_ENABLED: false

Review comment:
       Hm, now that you mention it... actually no... I was starting with a different idea and then I guess I forgot to remove them. Good catch!




-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#issuecomment-1003702901


   @ptuomola ... alright, added documentation and configuration checks... and did the requested cleanups.


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vidakovic commented on pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
vidakovic commented on pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#issuecomment-1003690871


   I'll update the README. Good idea with the startup validation, I like that idea.


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] ptuomola commented on a change in pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #2012:
URL: https://github.com/apache/fineract/pull/2012#discussion_r777170786



##########
File path: .github/workflows/build.yml
##########
@@ -47,5 +56,24 @@ jobs:
         run: |
             sudo apt-get update
             sudo apt-get install ghostscript -y
-      - name: Build & Test
+
+      - name: Basic Auth Build & Test
+        env:
+          - FINERACT_SECURITY_BASICAUTH_ENABLED: true
+          - FINERACT_SECURITY_OAUTH_ENABLED: false

Review comment:
       Do these variables actually do anything? It seems that we are hardcoding the same parameters in the command line when starting the server for oauth2-test and basictest etc? So it would seem that these are redundant... correct?

##########
File path: oauth2-tests/build.gradle
##########
@@ -45,7 +45,7 @@ cargo {
         }
         startStopTimeout = 240000
         containerProperties {
-            property 'cargo.start.jvmargs', '-Dspring.profiles.active=oauth'
+            property 'cargo.start.jvmargs', '-Dfineract.security.basicauth.enabled=false -Dfineract.security.oauth.enabled=true -Dfineract.security.2fa.enabled=false'

Review comment:
       Ie the environment is hardcoded here and these are the values used - correct?




-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] ptuomola merged pull request #2012: Simplify build for different auth strategies

Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #2012:
URL: https://github.com/apache/fineract/pull/2012


   


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org