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.

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

SSO support in UV #282

Merged
merged 58 commits into from
Mar 30, 2015
Merged

SSO support in UV #282

merged 58 commits into from
Mar 30, 2015

Conversation

skrchnavy
Copy link

Improved security/permission model in UV to support SSO.
Implementation shall be backward compatible, so if UV not configured to use SSO, it uses users stored in DB and also 2 roles are supported.

@tomas-knap
Copy link

@peterklimo, please adjust also mysql DB scripts based on your changes (table Permission etc). I cannot see such adjustment in the commits, although there is an adjustment for postgres: db/postgresql/data.sql

@peterklimo I miss some "cas_enabled=yes/no" config option in the configuration file. What is the logic which decides whether CAS will be used or not? It tries to use CAS configuration from config.properties and if it fails (CAS server cannot be contacted), non-CAS authentication is used? Or is there somewhere a special flag which directly says which type of authentication is supported (which is more safe - if CAS fails, error is reported and non-CAS authentication is not accepted)?

@tomas-knap
Copy link

@peterklimo Please answer my question above. For example, I still do not see updated db scripts for mysql. I cannot accept this pull request without updated scripts for mysql as well

@tomas-knap tomas-knap assigned peterklimo and unassigned tomas-knap Mar 27, 2015
@tomas-knap
Copy link

The only pending issues for the release:

Merge develop to feature/sso_fixes
Update script for mysql.

…sso_fixes

* commit 'deb5640dbba48db3bfbc6854b65ec3ae6b994330': (48 commits)
  Repaired MOD theme.
  #320. Refactoring.
  #320. Renamed method.
  #320. Replaced custom cryptography implementation with Jasypt on behalf of @tomas-knap and @eea04.
  Added description, why constraint is limited to smaller index.
  #320. Added cryptography.enabled setting on behalf of @tomas-knap.
  Removed incorrect, unnecessary statement.
  Added update script.
  Changed sample default language to EN.
  Typo in package name fix.
  Added unique constraint on name of pipeline in database.
  #320. Added cryptography support. Not mandatory, turned off by default, does not intercept export / import DPU nor copy pipeline.
  Added exported system packages for OSGi needed by JDBC drivers in DPUs
  Implementation of new relational data unit interface method for creating SQL connection for specific user
  Removing jrebel plugin from pom.xml
  Added getEnvironment into ConfigDialogContextImpl #327
  Obsolete buttons removed from dpu view. Requested by mrajniak a tknapp
  Fix: Added option to insert environment properties to test env.
  Revert "Added cryptography support, which is not mandatory and turned off by default."
  Revert "Renamed cryptography package."
  ...

Conflicts:
	backend/src/main/java/cz/cuni/mff/xrg/odcs/backend/context/Context.java
	commons-app/src/main/java/cz/cuni/mff/xrg/odcs/commons/app/conf/ConfigProperty.java
	db/mysql/schema.sql
	db/postgresql/updates/1.6.0-update.sql
	frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/pipelinelist/PipelineListViewImpl.java
	frontend/src/main/resources/frontend-messages.properties
	frontend/src/main/webapp/WEB-INF/config.sample.properties
@tomas-knap
Copy link

@peterklimo Please let me know when mysql update script is ready (we were discussing Sunday morning). Cannot prepare release branch without first merging feature/sso_fixes to develop (feature branches should not be merged to release branches directly) . And I cannot merge feature/sso_fixes to develop without the update script.

@tomas-knap
Copy link

@peterklimo One more issue, the following code (if (!defaultOrganization.isEmpty()) ) from PasswordAuthenticationProvider.java throws NullPointerException if ownership.type is set to user (in config properties) and organization is not specified in config.properties:

  String defaultOrganization = null;
        if (ORG_MODE.equals(appConfig.getString(ConfigProperty.OWNERSHIP_TYPE))) {
            defaultOrganization = appConfig.getString(ConfigProperty.DEFAULT_ORGANIZATION);
        }

        Organization o = null;

        if (!defaultOrganization.isEmpty()) {
            o = userFacade.getOrganizationByName(defaultOrganization);
            if (o == null) {
                o = new Organization();
                o.setName(defaultOrganization);
                userFacade.save(o);
            }
            user.setOrganization(o);
        }

Solution: if (defaultOrganization != null && !defaultOrganization.isEmpty()) {

@peterklimo
Copy link

i will close open issues today after 22.00

On March 29, 2015 6:36:46 PM CEST, Tomas Knap [email protected] wrote:

@peterklimo One more issue, the following code (if (!defaultOrganization.isEmpty()) ) from
PasswordAuthenticationProvider.java throws NullPointerException if
ownership.type is set to user (in config properties) and organization
is not specified in config.properties:

 String defaultOrganization = null;
if
(ORG_MODE.equals(appConfig.getString(ConfigProperty.OWNERSHIP_TYPE))) {
defaultOrganization =
appConfig.getString(ConfigProperty.DEFAULT_ORGANIZATION);
       }

       Organization o = null;

       if (!defaultOrganization.isEmpty()) {
           o = userFacade.getOrganizationByName(defaultOrganization);
           if (o == null) {
               o = new Organization();
               o.setName(defaultOrganization);
               userFacade.save(o);
           }
           user.setOrganization(o);
       }

Solution: if (defaultOrganization != null &&
!defaultOrganization.isEmpty()) {


Reply to this email directly or view it on GitHub:
#282 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@peterklimo
Copy link

fixed
update script created and pushed into repository

see e6820a0

On Sun, Mar 29, 2015 at 7:19 PM, Peter Klimo [email protected] wrote:

i will close open issues today after 22.00

On March 29, 2015 6:36:46 PM CEST, Tomas Knap [email protected]
wrote:

@peterklimo https://github.com/peterklimo One more issue, the
following code (if (!defaultOrganization.isEmpty()) ) from
PasswordAuthenticationProvider.java throws NullPointerException if
ownership.type is set to user (in config properties) and organization is
not specified in config.properties:

String defaultOrganization = null;
if (ORG_MODE.equals(appConfig.getString(ConfigProperty.OWNERSHIP_TYPE))) {
defaultOrganization = appConfig.getString(ConfigProperty.DEFAULT_ORGANIZATION);
}

    Organization o = null;

    if (!defaultOrganization.isEmpty()) {
        o = userFacade.getOrganizationByName(defaultOrganization);
        if (o == null) {
            o = new Organization();
            o.setName(defaultOrganization);
            userFacade.save(o);
        }
        user.setOrganization(o);
    }

Solution: if (defaultOrganization != null &&
!defaultOrganization.isEmpty()) {


Reply to this email directly or view it on GitHub
#282 (comment).

Sent from my Android device with K-9 Mail. Please excuse my brevity.

tomas-knap added a commit that referenced this pull request Mar 30, 2015
SSO support in UV, permissions/roles
@tomas-knap tomas-knap merged commit a7bf6bc into develop Mar 30, 2015
@skrchnavy skrchnavy deleted the feature/sso_fixes branch March 31, 2015 04:42
@peterklimo peterklimo restored the feature/sso_fixes branch April 28, 2015 09:25
@skrchnavy skrchnavy deleted the feature/sso_fixes branch June 4, 2015 08:38
@peterklimo peterklimo restored the feature/sso_fixes branch June 16, 2015 14:04
@skrchnavy skrchnavy deleted the feature/sso_fixes branch June 19, 2015 14:57
@peterklimo peterklimo restored the feature/sso_fixes branch June 24, 2015 09:41
@skrchnavy skrchnavy deleted the feature/sso_fixes branch June 25, 2015 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants