enables support for feign clients using OAuth2 client credentials grant #3662

xetys · 2016-05-29T19:53:01Z

so feign clients can be added declarative to. This is a requirement for the next step of a JHipster feign client subgenerator.

contents of this PR

adds dependencies to microservice applications
extends JHipster properties to configure client credentials for internal communications
replaces hardcoded internal client with above configuration in UAA
configures a OAuth2ProtectedRessourceDetails based on new configuration and declares a OAuth2RequestInterceptor bean
enables with @EnableFeignClients the usage of feign clients in microservice applications

so far this gets merged, I am going to proceed with the feign client generators

to having both UAA and microservices access to the internal client credentials for OAuth, lately to be used by feign clients for interservice communication

so feign clients can be added declarative to. This is a requirement for the next step of a JHipster feign client subgenerator. for feature request jhipster#3649

PierreBesson · 2016-05-30T08:01:26Z

I think this is the right approach for inter-service communication. 👍 @xetys.
However shouldn't this be also enabled for gateway apps ?

so an access token can be resolved using ribbon and eureka instead of an url

xetys · 2016-05-31T15:30:58Z

after hours of total frustration and realizing, there is no offical support to loadbalance the access token uri before acquiring access tokens....here comes an (maybe not fully elegant update), which solves the problem in its root...

the last 2 commits from this comment contain a workaround, so the URL is balanced with eureka and ribbon everytime something is asking for the url. This should only happen, when FeignClients are used.

from this point this PR is complete for me, please review as soon as possible, so I can proceed with a feign-client generator

PierreBesson · 2016-06-01T08:44:10Z

generators/server/templates/src/main/java/package/config/_LoadBalancedResourceDetails.java

+
+                return newUrl;
+            } catch (URISyntaxException e) {
+                e.printStackTrace();


Please use a logger call here instead of printStackTrace.

PierreBesson · 2016-06-01T08:53:09Z

I think this PR can be merged after someone else than me have reviewed it. It concerns only microservices with UAA.
@xetys, I would appreciate if you could upload a sample project, where we can test secured Feign calls, circuit breakers, load balancing, etc... If it does not take too much of your time.

deepu105 · 2016-06-01T16:38:59Z

A bit busy right now. Ill take a look as soon as I have time

Thanks & Regards,
Deepu

On Wed, Jun 1, 2016 at 4:53 PM, Pierre Besson notifications@github.com
wrote:

I think this PR can be merged after someone else than me have reviewed it.
It concerns only microservices with UAA.
@xetys https://github.com/xetys, I would appreciate if you could upload
a sample project, where we can test secured Feign calls, circuit breakers,
load balancing, etc...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#3662 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABDlF6MxJzHtsQeE7DHTu_Kw_Y9xxRvUks5qHUh8gaJpZM4IpXPs
.

xetys · 2016-06-01T17:31:30Z

same here

@PierreBesson basicly you can checkout https://github.com/xetys/jhipster-uaa-setup/commits/master
its not clean yet and is containing some experiments in app1, app2 is purely generated using my branch..

the failing build i will checkout as soon as i find time

xetys · 2016-06-04T10:39:31Z

Ok now I found some time to provide slightly more information

This PR allows you to do the following out of the box:
In a example micrroservice with services "app1, serving /api/foos" and "app2, serving /api/baars" you want app2 to have a client for resolving "foos" from app1.

During configuration, you decided to give the UAA the name "uaa" and port 9999 (so you can keep the default configuration unchanged)

in app2 you add a Foo.class

public class Foo {
    private long id;

    private String value;

    public String getValue() {
        return value;
    }

    public void setValue(String value) {
        this.value = value;
    }

    public long getId() {
        return id;
    }

    public void setId(long id) {
        this.id = id;
    }
}

and FooClient.java

@FeignClient("http://app1")
public interface FooClient {
    @RequestMapping("/api/foos")
    List<Foo> getFoos();
}

to enable the client. You may use it to serve in an app2 RestController for testing purpose like this:

@RestController
@RequestMapping("/api/clients/foo")
public class FooClientResource {

    private FooClient fooClient;

    @Autowired
    public FooClientResource(FooClient fooClient) {
        this.fooClient = fooClient;
    }

    @RequestMapping(method = RequestMethod.GET)
    public ResponseEntity<List<Foo>> getFoos() {
        return ResponseEntity.ok(fooClient.getFoos());
    }
}

app2 will now consume foos from app1, with an seperate "session" for OAuth2 client internal, rather then the users session. Additionally both "app1" and "uaa" are load balanced using ribbon and eureka client.

Note, that there is no official solution how to make spring-cloud-feign resovle the UAA via ribbon, so this PR provides a workaround (approved by Dave Syer 😄 ), to let feign also resolve your JHipster UAA (or any other). To apply this configuration in detail, you modify application.yml (or -dev / -prod)

jhipster:
       # ...
        clientAuthorization:
            tokenUrl: http://MyUaa:9999/oauth/token #your custom uaa
            tokenServiceId: MyUaa # eurekas service name
            clientId: internal
            clientSecret: internal

you can view this example here

hope that helps you to test....I will wrap all this is merged into a documentation and article as well

deepu105 · 2016-06-09T07:57:55Z

generators/server/index.js

@@ -578,6 +578,9 @@ module.exports = JhipsterServerGenerator.extend({
            if (this.applicationType !== 'microservice' && !(this.applicationType === 'gateway' && this.authenticationType === 'uaa')) return;

            this.template(SERVER_MAIN_SRC_DIR + 'package/config/_MicroserviceSecurityConfiguration.java', javaDir + 'config/MicroserviceSecurityConfiguration.java', this, {});
+            if(this.applicationType === 'microservice' && this.authenticationType === 'uaa') {


space between if (

deepu105 · 2016-06-09T08:06:06Z

This looks good. I have some minor comments, mostly aesthetic. I guess its good to merge once those comments are taken care of

cbornet · 2016-06-17T12:40:58Z

generators/server/templates/_build.gradle

@@ -207,6 +207,10 @@ dependencies {
    compile "org.springframework.cloud:spring-cloud-starter-config"
    compile "org.springframework.retry:spring-retry"
    <%_ } _%>
+    <%_ if (applicationType === 'microservice' && authenticationType === 'uaa') { _%>


Feign clients are interesting even without uaa so I would remove the uaa check

xetys · 2016-09-07T16:40:08Z

found: maybe removing @ComponentScan globally was not the best idea 😆

i fixed that in the branch

jdubois · 2016-09-09T06:58:35Z

OK great.
Not sure I'll have the time to test as this is quite big.
If not, it will be in another release, but holidays are over and I'd like
to have more frequent releases, so that won't take long.

Le 7 sept. 2016 6:40 PM, "David Steiman" notifications@github.com a
écrit :

found: maybe removing @componentscan globally was not the best idea 😆

i fixed that in the branch

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3662 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AATVo2XvBZ8fXJmIK9gcdS_U1glnQufTks5qnujqgaJpZM4IpXPs
.

xetys · 2016-09-09T07:39:05Z

sure, ask if there's something more...

ValerioZ · 2016-09-09T15:01:18Z

Sorry, I cannot find @AuthorizedFeignClient. I cannot perform any request by Feign to a protected resource with jhipster module in the marketplace.
Have you any idea of why?
Thanks

xetys · 2016-09-09T15:09:31Z

@ValerioZ did you checked out my specific branch, or using the official release....this is currently not even merged to master branch

Conflicts: generators/server/templates/src/main/java/package/config/_MicroserviceSecurityConfiguration.java

jdubois · 2016-09-11T08:30:27Z

@xetys I am testing on the https://github.com/jhipster/generator-jhipster/tree/xetys-feign-clients and I'm going to merge this if it's all OK.

I just have one issue at the moment: I generated a microservice, with one entity, and its "./mvnw test" do not pass, as it's looking for a UAA server. So you need to mock this, or provide a specific "test" profile to solve this. This is not a blocker for me, at the moment.

jdubois · 2016-09-11T08:35:19Z

@xetys sorry but I put everything in Docker, and both my gateway and microservice failed, with error:

Caused by: org.springframework.beans.NotWritablePropertyException: Invalid property 'security.client-authorization[accessTokenUri]' of bean class [io.github.jhipster.bbl.config.JHipsterProperties]: Cannot access indexed value in property referenced in indexed property path 'client-authorization[accessTokenUri]'; nested exception is org.springframework.beans.NotReadablePropertyException: Invalid property 'security.client-authorization[accessTokenUri]' of bean class [io.github.jhipster.bbl.config.JHipsterProperties]: Bean property 'security.client-authorization[accessTokenUri]' is not readable or has an invalid getter method: Does the return type of the getter match the parameter type of the setter?

Both of them are configured to connect the UAA server, of course.

Sorry but I can't merge it right now, it's not working :-(

xetys · 2016-09-11T08:56:07Z

I will have time to take a look in one hour. I think there is some file just missing somewhere, which I need to find.

So what do you think about the Bean exclusion annotation, keep it or use direct type exclusion on component scan instead?

jdubois · 2016-09-11T09:02:54Z

I think the Bean exclusion could be good, but it should be in another PR, so we don't mix ideas/concerns. And I still don't understand why you want to exclude that Bean, but if you do I think you can just exclude it directly by type (I'm not 100% sure here, as I'm not sure of why you do it)

xetys · 2016-09-11T09:06:55Z

As I explained, we want to leave the option open for developers, to use feign clients generally, without the uaa stuff. If we don't exclude, the OAuth request interceptor gets injected to all clients. This behaviour is described in official spring docs.

…n-clients

Conflicts: generators/server/templates/src/main/java/package/config/_MicroserviceSecurityConfiguration.java

xetys · 2016-09-11T11:58:42Z

@jdubois I regenerated my JHipster UAA setup using this branch and do not expierienced problems using docker...

I uploaded here my example, so you can try my setup

you can use my docker-compose setup

jdubois · 2016-09-11T15:37:32Z

@xetys I merged it in https://github.com/jhipster/generator-jhipster/tree/xetys-feign-clients and corrected a few things (there was a compilation error for gateway & uaa). When I run it, I still have a error on the gateway:

Bean property 'security.client-authorization[accessTokenUri]' is not readable or has an invalid getter method: Does the return type of the getter match the parameter type of the setter? -> [Help 1]

jdubois · 2016-09-11T15:45:39Z

I don't understand what's going on:

Some of the stuff from this PR looks like being already in the master branch
And the master branch is broken, both the microservice and gateway fail

Looks like I pushed stuff by mistake in the master branch, I'm totally lost here... That's bad, I wanted to do the 3.7.0 this evening as I won't have much time this week...

jdubois · 2016-09-11T15:51:43Z

OK, I understand, this is because there is #4052 which is in both branches, and which was merged by @deepu105 today!
I might as well merge all this to master:

I'll close this ticket when this merged (I'm merging the branch on generator-jhipster)
Still there will be the issue for the gateway

deepu105 · 2016-09-11T16:24:56Z

@xetys next time try not to mix up commits from different PRs together. This has a lot of unwanted commits and it messes up the commit history on master

ValerioZ · 2016-09-12T05:56:14Z

@xetys I'm using official 3.6.1 release with beta UAA service. Thanks.

xetys · 2016-09-12T08:40:54Z

@ValerioZ as you can see in this PR, this feature is currently in release progress and is scheduled for 3.7, and is not available to prior releases. Update to 3.7 as soon it is out, to use features like @AuthorizedFeignClient

Please note, that your question would be placed better in stackoverflow, rather then in this discussion.

xetys added 5 commits May 29, 2016 18:08

fixes "internal" OAuth2 client and adds JHipster configuration

52ee4e5

to having both UAA and microservices access to the internal client credentials for OAuth, lately to be used by feign clients for interservice communication

applies the new JHipster configuration

fa06f12

adds feign and spring-cloud-security to microservice-generator

d67ca49

fixes silly syntax error in generator

1d0be48

enables support for feign clients using OAuth2 client credentials grant

8bd33a3

so feign clients can be added declarative to. This is a requirement for the next step of a JHipster feign client subgenerator. for feature request jhipster#3649

David Steiman added 2 commits May 31, 2016 17:21

adds loadbalancing support for uaa resolve

4c3fff0

so an access token can be resolved using ribbon and eureka instead of an url

adds the file into generator process

d5955c9

fixes indent

b553669

PierreBesson reviewed Jun 1, 2016
View reviewed changes

xetys added 2 commits June 4, 2016 11:43

restricts LoadBalancedResourceDetails for UAA services only

5a13550

updates the propper client auth configuration

3f08fa9

adds logger instead of printing stacktrace

b66850a

deepu105 reviewed Jun 9, 2016
View reviewed changes

deepu105 added the pr: needs-change label Jun 9, 2016

cbornet reviewed Jun 17, 2016
View reviewed changes

xetys mentioned this pull request Jul 1, 2016

make components, working with feign, testable #3797

Closed

xetys added 3 commits September 10, 2016 23:23

replaces profile fix with adding overriding primary beans to test root

a8e56e4

Merge remote-tracking branch 'upstream/master' into fixing-3982

2026ef8

Conflicts: generators/server/templates/src/main/java/package/config/_MicroserviceSecurityConfiguration.java

fix failed merge

08ff457

xetys added 5 commits September 11, 2016 11:33

exclude uaa related test support for non uaa apps

22fc857

Merge remote-tracking branch 'upstream/xetys-feign-clients' into feig…

b02ba3b

…n-clients

Merge remote-tracking branch 'upstream/master' into feign-clients

719399b

Conflicts: generators/server/templates/src/main/java/package/config/_MicroserviceSecurityConfiguration.java

fixes missing file copy

6fe4a2a

Merge branch 'fixing-3982' into feign-clients

b996f40

exlude the request interceptor by type-exclusion in app class

0c61d1c

jdubois merged commit 0c61d1c into jhipster:master Sep 11, 2016

jdubois modified the milestone: v3.7.0 Sep 11, 2016

xetys deleted the feign-clients branch September 12, 2016 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enables support for feign clients using OAuth2 client credentials grant #3662

enables support for feign clients using OAuth2 client credentials grant #3662

xetys commented May 29, 2016

PierreBesson commented May 30, 2016

xetys commented May 31, 2016

PierreBesson Jun 1, 2016

PierreBesson commented Jun 1, 2016 •

edited

Loading

deepu105 commented Jun 1, 2016

xetys commented Jun 1, 2016

xetys commented Jun 4, 2016

deepu105 Jun 9, 2016

deepu105 commented Jun 9, 2016

cbornet Jun 17, 2016 •

edited

Loading

xetys commented Sep 7, 2016

jdubois commented Sep 9, 2016

xetys commented Sep 9, 2016

ValerioZ commented Sep 9, 2016

xetys commented Sep 9, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

xetys commented Sep 11, 2016

jdubois commented Sep 11, 2016

xetys commented Sep 11, 2016

xetys commented Sep 11, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

deepu105 commented Sep 11, 2016

ValerioZ commented Sep 12, 2016

xetys commented Sep 12, 2016

enables support for feign clients using OAuth2 client credentials grant #3662

enables support for feign clients using OAuth2 client credentials grant #3662

Conversation

xetys commented May 29, 2016

contents of this PR

PierreBesson commented May 30, 2016

xetys commented May 31, 2016

PierreBesson Jun 1, 2016

Choose a reason for hiding this comment

PierreBesson commented Jun 1, 2016 • edited Loading

deepu105 commented Jun 1, 2016

xetys commented Jun 1, 2016

xetys commented Jun 4, 2016

deepu105 Jun 9, 2016

Choose a reason for hiding this comment

deepu105 commented Jun 9, 2016

cbornet Jun 17, 2016 • edited Loading

Choose a reason for hiding this comment

xetys commented Sep 7, 2016

jdubois commented Sep 9, 2016

xetys commented Sep 9, 2016

ValerioZ commented Sep 9, 2016

xetys commented Sep 9, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

xetys commented Sep 11, 2016

jdubois commented Sep 11, 2016

xetys commented Sep 11, 2016

xetys commented Sep 11, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

jdubois commented Sep 11, 2016

deepu105 commented Sep 11, 2016

ValerioZ commented Sep 12, 2016

xetys commented Sep 12, 2016

PierreBesson commented Jun 1, 2016 •

edited

Loading

cbornet Jun 17, 2016 •

edited

Loading