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

Upgrade to pac4j v1.9.2 #438

Closed
leleuj opened this issue Jul 25, 2016 · 10 comments
Closed

Upgrade to pac4j v1.9.2 #438

leleuj opened this issue Jul 25, 2016 · 10 comments

Comments

@leleuj
Copy link
Contributor

leleuj commented Jul 25, 2016

Hi,

pac4j v1.9.1 is out and Jooby should upgrade to use it.

The source code has been cleaned (-15% in size), dependencies (Java 8 also) have been upgraded, multi-profiles are supported and extension capabilities are much better -> https://github.com/pac4j/pac4j/wiki/Versions

Keeping the current Jooby security algorithms and upgrading to pac4j v1.9.1 should be straightforward (most API signatures have remained the same). This is your option number 1: really easy!


Though, pac4j has reached a strong maturity with version 1.9: the security model is now available via specific components (and not only guidelines).
In addition to the concepts: clients, authorizers and matchers (whether to apply security or not), you now have three "filters":

  1. The "security filter" (or whatever the mechanism used to intercept HTTP requests) protects an url by checking that the user is authenticated and that the authorizations are valid, according to the clients and authorizers configuration. If the user is not authenticated, it performs authentication for direct clients or starts the login process for indirect clients
  2. The "callback controller" finishes the login process for an indirect client
  3. The application logout controller" logs out the user from the application.

For example, for the "security filter", its logic is available via the DefaultSecurityLogic which makes implementations very easy:

  • In J2E:
    @Override
    protected final void internalFilter(final HttpServletRequest request, final HttpServletResponse response,
                                        final FilterChain filterChain) throws IOException, ServletException {

        assertNotNull("securityLogic", securityLogic);

        final Config config = getConfig();
        assertNotNull("config", config);
        final J2EContext context = new J2EContext(request, response, config.getSessionStore());

        securityLogic.perform(context, config, (ctx, parameters) -> {

            filterChain.doFilter(request, response);
            return null;

        }, J2ENopHttpActionAdapter.INSTANCE, clients, authorizers, matchers, multiProfile);
    }
  • In Play:
    public CompletionStage<Result> internalCall(final Context ctx, final String clients, final String authorizers, final boolean multiProfile) throws Throwable {

        assertNotNull("config", config);
        final PlayWebContext playWebContext = new PlayWebContext(ctx, sessionStore);
        final HttpActionAdapterWrapper actionAdapterWrapper = new HttpActionAdapterWrapper(config.getHttpActionAdapter());

        return securityLogic.perform(playWebContext, config, (webCtx, parameters) -> {
            // when called from Scala
            if (delegate == null) {
                return CompletableFuture.completedFuture(null);
            } else {
                return delegate.call(ctx);
            }
        }, actionAdapterWrapper, clients, authorizers, null, multiProfile, ctx);
    }

And the ProfileManager is the component to use to get the current authenticated user while the SessionStore is the abstraction for the web session.

The short guide: https://github.com/pac4j/pac4j/wiki/How-to-implement-pac4j-for-a-new-framework---tool

So this is your option number 2: a lot more work, but a lot more powerful: multiple clients / authorizers definitions, multi profiles authenticated at the same time...


What do you think?

Thanks.
Best regards,
Jérôme

@leleuj
Copy link
Contributor Author

leleuj commented Sep 12, 2016

Any update on this? Do you want me to submit a first simple upgrade?

@jknack
Copy link
Member

jknack commented Sep 12, 2016

Hi @leleuj

Sorry, I've been busy with the upcoming release 1.0.0. (we are finally there).
It will great if you can submit an upgrade!!

@leleuj
Copy link
Contributor Author

leleuj commented Sep 13, 2016

OK. I will handle it. Taking a first look at the code, I don't think option 2 (new model) really make sense for version 1.0.0: you have a very interesting codebase using most of the pac4j components in your way. I will upgrade them, it should be straightforward.

@leleuj
Copy link
Contributor Author

leleuj commented Sep 14, 2016

I have prepared the following PR: #477

The changes are fairly straightforward:

  1. HttpProfile no longer exists and UserProfile is now abstract and no more used in signatures: so the CommonProfile replaces them
  2. Some package names have changed
  3. the UsernamePasswordAuthenticator has been removed, the Authenticator<UsernamePasswordCredentials> must be used instead
  4. the version 1.9.2 of pac4j is used.

I have fixed some errors in the documentation and added a link to the clients doc page.

Nonetheless, I have some issues:

  1. I don't see any reference to the AuthorizerFilter in the doc: I think it's missing
  2. The pom.xml file references a version 1.0.0.Final: I cannot build the project
  3. All tests pass except two in the AuthFilterTests: when mocking a ParameterClient, the internal logger property is null and generates a NullPointerException.

I guess it should be easy for you to fix the remaining issues: just let me know...

@leleuj
Copy link
Contributor Author

leleuj commented Sep 14, 2016

The build fails: it seems I missed some classes in the coverage-report directory. Working on this...

@jknack
Copy link
Member

jknack commented Sep 14, 2016

Thank you @leleuj!

  1. will look
  2. Try first a mvn clean install -DskipTests at root/parent pom.xml/dir that will generate 1.0.0.Final (which is the next release)
  3. will look

Thanks again.

@jknack jknack added this to the 1.0.0 milestone Sep 14, 2016
@leleuj
Copy link
Contributor Author

leleuj commented Sep 14, 2016

I'm used to see -SNAPSHOT versions in master, but anyway I just pushed a new commit (it works locally using : mvn clean install -DskipTests).

@jknack
Copy link
Member

jknack commented Sep 14, 2016

I used to do the same in the past and other projects.. still not sure why I set 1.0.0 and .Final (too lazy perhaps ) after 1.0.0. release will get back to the -SNAPSHOT notation.

@leleuj
Copy link
Contributor Author

leleuj commented Sep 14, 2016

The compilation works now, just the two tests fail:

Tests in error: 
  AuthFilterTest.handleDirectNoProfile »  Unexpected exception, expected<org.joo...
  AuthFilterTest.handleDirect:219->lambda$userProfile$13:139 » NullPointer

@jknack jknack changed the title Upgrade to pac4j v1.9.1 Upgrade to pac4j v1.9.2 Sep 16, 2016
@jknack
Copy link
Member

jknack commented Sep 16, 2016

Build is now fixed.

Thanks

@jknack jknack closed this as completed Sep 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants