hstaudacher / osgi-jax-rs-connector

An OSGi - JAX-RS 2.0 Connector, software repository available on the link below
http://hstaudacher.github.io/osgi-jax-rs-connector
Other
191 stars 98 forks source link

Fix for #16 - Provide an OSGi friendly security framework #20

Closed BryanHunt closed 10 years ago

BryanHunt commented 10 years ago

The example and unit test works. The unit test had to be done as an integration test and has a separate launcher. I hacked the pom files, but did not try to get them working.

hstaudacher commented 10 years ago

I'll find time on friday for a review. In the meantime do you have any idea why the build fails for it?

BryanHunt commented 10 years ago

I hacked the pom files ... if you have instructions on how to test the build locally, I'll see if I can fix what is broken.

On Oct 22, 2013, at 11:21 AM, Holger Staudacher notifications@github.com wrote:

I'll will find time on friday for a review. In the meantime do you have any idea why the build fails for it?

— Reply to this email directly or view it on GitHub.

hstaudacher commented 10 years ago

Either install the m2e tooling and use the launch config: https://github.com/hstaudacher/osgi-jax-rs-connector/blob/master/build/com.eclipsesource.jaxrs.build/JAX-RS%20Connector%20Build.launch

or step into /build/com.eclipsesource.jaxrs.build/ folder and execute mvn clean verify

hstaudacher commented 10 years ago

Added the instructions to the wiki :)

https://github.com/hstaudacher/osgi-jax-rs-connector/wiki/FAQ

BryanHunt commented 10 years ago

I fixed the build, but the security test is not working with maven. It works fine if you run it from the launcher. I commented out the security test module from the build pom. This is one I could probably use some help on.

hstaudacher commented 10 years ago

Hi Bryan, I have reviewed the changes and I did some minor refactoring. I couldn't push it to master because from my point of view I think one core feature does not work. Please correct me if I'm wrong.

I saw that the the TestResource has a "secure" and a "public" method. I assume this would mean that I can access the public method without authentication, right? Sadly this does not work. When the security provider is started all services need authentication. This is also true for services in another bundle. I have added the connector.example to the launch config and wanted to access http://localhost:9090/services/osgi-jax-rs which has also returned a 401.

I have pushed everything to a branch named security. Can you take a second look if I'm right?

BryanHunt commented 10 years ago

This latest commit should fix the security problems with the public resource.

BryanHunt commented 10 years ago

Ping

hstaudacher commented 10 years ago

Pong :). Sorry, very busy. I'm trying hard to find time this week.

hstaudacher commented 10 years ago

Sorry @BryanHunt, but the public services are still not working without authentication. Also I noticed you didn't picked up the security branch. Something wrong with it?

BryanHunt commented 10 years ago

Did you pick up my latest code in the master branch? I added the connector.example to the security example launcher and was able to access http://localhost:9090/services/osgi-jax-rs without authentication.

client

BryanHunt commented 10 years ago

To double check, make sure the SecurityService in the security.example bundle has the following code:

 @Override
  public Principal authenticate(ContainerRequestContext requestContext) {
    Cookie userCookie = requestContext.getCookies().get("user");
    Cookie authCookie = requestContext.getCookies().get("auth");

    if(userCookie == null || authCookie == null || !userCookie.getValue().equals(authCookie.getValue()))
      return new User("");

    return new User(userCookie.getValue()); 
  }
hstaudacher commented 10 years ago

Hi Bryan, I have integrated the change now. Sorry it took that long. I have done some refactoring and writing tests. Hope you agree with the changes. See commit e6161e768fcb65ab7ddb226810b43fc8594a1809

hstaudacher commented 10 years ago

I have also created a wiki page: https://github.com/hstaudacher/osgi-jax-rs-connector/wiki/security

BryanHunt commented 10 years ago

I'm fine with the changes except for one thing. For the code I wrote, I legally own the copyright yet you changed it to EclipseSource. Unless we agree to joint copyright ownership, you can't do that.

One other minor issue ... I think SecurityContextImpl should default to returning false if there is no authorization handler. If a developer accidentally deployed without enabling the authorization handler, there would be no security.

hstaudacher commented 10 years ago

Hi Bryan, I thought I just added myself as a contributor. Can you name the classes or just change it yourself? Was not by purpose.

BryanHunt commented 10 years ago

I'm happy to submit a pull request. How about the security issue? If you agree with my change, I'll put it in the same commit.

hstaudacher commented 10 years ago

Agree ;)