[Api Gateway] Support Authentication with OpenStack KeyStone #219

Gzure · 2020-05-27T08:13:55Z

Spring-cloud-starter-gateway can't use with spring-boot-starter-web, Because sprint application will start to default Webservlet, but sprint-cloud-starter-gateway need start reactiveServlet, it will throw an
"No qualifying bean of type 'org.springframework.core.convert.ConversionService' available" error.

xieus

Thanks @Gzure. Some early feedback.

...s/api_gateway/src/main/java/com/futurewei/alcor/apigateway/filter/KeystoneAuthWebFilter.java

xieus

Thanks @Gzure. Some early feedback.

Gzure · 2020-05-29T01:07:48Z

@xieus I add an integration_keystone.adoc, please take a look.

xieus · 2020-05-29T03:17:57Z

@xieus I add an integration_keystone.adoc, please take a look.

Sure. Thanks for making the change. I am on it.

xieus

Good doc! Leave some early feedback. Thanks @Gzure

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc

Gzure · 2020-05-30T03:25:15Z

I defined a token entity which is used for cache.
It includes some fields:

string token: a token self
date expireAt: the date which token expire at for judge the token whether valid
string user the token user name
string userId: the token user id
string domainId: the token user domain id
string domainName: the token user domain name
string projectNmae: the token project name
string projectId: the token project id
string projectDomain: the token project domain
boolean invalid: the token whether valid from keystone, true: invalid false: valid
list roles: the token user role names, this field can use for user authentication
method boolean isExpired: for judge this token whether pass authentication
@xieus Could you take a look at this or make some comments?

xieus · 2020-05-30T04:54:00Z

It includes some fields:

Thanks. I would recommend to add those field in the section of "data schema" in the design doc. Also includes an example.

date expireAt: the date which token expire at for judge the token whether valid

For expireAt, please use UTC ISO8601 format.

string user the token user name

string userId: the token user id

string domainId: the token user domain id

string domainName: the token user domain name

string projectNmae: the token project name

string projectId: the token project id

Looks good. The token is used in a project scale, right?

string projectDomain: the token project domain

What is "projectDomain"?

boolean invalid: the token whether valid from keystone, true: invalid false: valid

What is the variable name?

list roles: the token user role names, this field can use for user authentication

What is the variable name?

method boolean isExpired: for judge this token whether pass authentication

Gzure · 2020-05-30T06:33:27Z

ExpireAt uses UTC ISO8601 format now.
Keystone V3 add a new scope domain, is projects container, a domain can have multi projects
The token have a few of scopes, such as domain, project.. , if the token in project scope that will have project id. Most cases the token is in project scope
Project domain is that which domain belong
The invalid is the result from keystone if verify pass, the invalid is false, other is false
The roles are the token corresponds to all user permission rules

…e.adoc

Gzure · 2020-06-03T08:29:52Z

About the write-through cache implementation, Please see commit 5a5473e.

xieus · 2020-06-03T18:12:27Z

@cj-chung, feel free to review and add comments.

xieus

Left a few more comments. Sorry it is late here. I will continue to review the rest tomorrow.

lib/src/main/java/com/futurewei/alcor/common/entity/TokenEntity.java

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc

cj-chung

looks good to me.

xieus · 2020-06-05T01:45:16Z

lib/src/main/java/com/futurewei/alcor/common/db/CacheFactory.java


-    public <K, V> ICache getRedisCache(Class<V> v, String cacheName) {
-        RedisTemplate<K, V> template = new RedisTemplate<K, V>();
+    public <K, V> RedisTemplate<K, V> getRedisTemplate(Class<V> v){


As this is a general interface for Ignite, Redis and other DBs, ideally we should keep the interface db-independent. Could we move this method to Redis class only?

@Gzure @chenpiaoping We need to add a comment to this interface. Thank you.

I think we could create a CacheFactory interface, and a DB interface implement a CacheFactory. So we will return different DB CacheFactory from application properties. We can make all DB keep independent.

lib/src/main/java/com/futurewei/alcor/common/db/CacheFactory.java

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc

xieus · 2020-06-05T02:20:40Z

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc

+. Add keystone conf in application.properties file (https://docs.openstack.org/keystone/latest/user/supported_clients.html[keystone config example])
+
+== KeystoneAuthWebFilter Start Workflow
+image::keystone_filter_start_workflow.jpg["KeystoneAuthWebFilter Start Workflow", width=1024, link="keystone_filter_start_workflow.jpg"]


Okay you refer to storing the Alcor Token in the API gateway memory, right? It is okay for now. But I was wondering we could store the Alcor token in the cache (maybe separate cache repo from the customer token).

Gzure · 2020-06-05T03:22:32Z

@xieus About the Alcor Token store in the cache. I guess you want multi API gateway instances to share one Alcor Token that can reduce request times to keystone. But there is a problem which multi instances get the token expired, send a create Token request to keystone at the same time. It may cause synchronization problems. Nonetheless, stored the Alcor Token in the cache may be better than stored locally.

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteCache.java

lib/src/main/java/com/futurewei/alcor/common/db/redis/RedisExpireCache.java

xieus · 2020-06-05T02:37:45Z

lib/src/main/java/com/futurewei/alcor/common/db/redis/RedisExpireCache.java

+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+
+public class RedisExpireCache<K, V> implements ICache<K, V> {


Like this class!

xieus · 2020-06-05T02:51:58Z

services/api_gateway/src/main/resources/application.properties

+keystone.project_name=service
+keystone.user_domain_name=Default
+keystone.username=xxxxx
+keystone.password=xxxxxx


I was a little worries about putting an password here as the password is a keystone is critical infra component. Spring should have some mechanism to prevent writing an explicit password in a configuration file.

@cj-chung Feel free to weigh in.

@Gzure This is not blocking for now.

@cj-chung @Gzure Tracked by Issue #242

services/api_gateway/src/main/java/com/futurewei/alcor/apigateway/utils/KeystoneClient.java

xieus · 2020-06-05T03:42:46Z

...ices/api_gateway/src/test/java/com/futurewei/alcor/apigateway/KeystoneAuthWebFilterTest.java

+
+@RunWith(SpringRunner.class)
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+        properties = {"httpbin=http://localhost:${wiremock.server.port}",


@Gzure As the code logics in KeyStoneAUthWebFilter and KeystoneClient are not simple, we would like to have UT coverage if possible? Ideally UTs should cover the happy path but the most important, the sad path with different unexpected input?

...ices/api_gateway/src/test/java/com/futurewei/alcor/apigateway/KeystoneAuthWebFilterTest.java

xieus · 2020-06-05T03:51:21Z

@xieus About the Alcor Token store in the cache. I guess you want multi API gateway instances to share one Alcor Token that can reduce request times to keystone. But there is a problem which multi instances get the token expired, send a create Token request to keystone at the same time. It may cause synchronization problems. Nonetheless, stored the Alcor Token in the cache may be better than stored locally.

Every instance could have its own token to avoid the synchronization issue. One instance one token could increases the API gateway availability as one token expiration doesn't cause outage of control plane.

When storing in the cache, each instance could have its own instance id, and use this instance id to query its own token. The question would be, how we generate the id and where we will store it? If stored in the memory, whenever the instance restarts, we will lose the id.

xieus

This PR is in good shape. Left a few more comments for you to take a look. Thank you, @Gzure

xieus · 2020-06-05T03:55:01Z

services/api_gateway/src/main/java/com/futurewei/alcor/apigateway/utils/KeystoneClient.java

+        getLocalToken();
+    }
+
+    public void checkEndPoints() throws IOException{


Could we make checkEndpoints and getLocalToken private?

services/api_gateway/src/main/java/com/futurewei/alcor/apigateway/utils/KeystoneClient.java

xieus · 2020-06-05T04:28:19Z

services/api_gateway/src/main/java/com/futurewei/alcor/apigateway/utils/KeystoneClient.java

+                Date expireDate = dateFormat.parse(expireDateStr);
+                te.setExpireAt(expireDate);
+
+                if(tokenNode.has("roles")){


Can we give an OpenStack KeyStone link about different roles to improve the readability?

xieus · 2020-06-05T04:35:43Z

...ices/api_gateway/src/test/java/com/futurewei/alcor/apigateway/KeystoneAuthWebFilterTest.java

+
+@RunWith(SpringRunner.class)
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+        properties = {"httpbin=http://localhost:${wiremock.server.port}",


@Gzure As the code logics in KeyStoneAUthWebFilter and KeystoneClient are not simple, we would like to have UT coverage if possible? Ideally UTs should cover the happy path but the most important, the sad path with different unexpected input?

xieus

Overall this PR is in high quality. It contributes to the Alcor project in three folds：

Integration with OpenStack KeyStone IDM, with detailed design and implementation
Refactor the cache factory and support new expire cache for token storage
Add detailed comments to a few critical classes/methods

Approve with pleasure :-)

xieus · 2020-06-09T15:06:36Z

lib/src/main/java/com/futurewei/alcor/common/db/CacheFactory.java

    }

-    public <K, V> ICache getRedisCache(Class<V> v, String cacheName) {
-        RedisTemplate<K, V> template = new RedisTemplate<K, V>();


xieus · 2020-06-09T15:22:51Z

services/api_gateway/src/main/java/com/futurewei/alcor/apigateway/client/KeystoneClient.java

+    }
+
+
+    /**


xieus · 2020-06-09T15:23:32Z

lib/src/main/java/com/futurewei/alcor/common/db/redis/RedisExpireCache.java

+    private RedisTransaction transaction;
+
+    /**
+     * return a new redis cache client with auto set expire time


Gzure added 2 commits May 25, 2020 11:19

fix a bug that apiGateway start error

61aa663

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

63f1968

Gzure changed the title ~~fix a bug that apiGateway start error~~ [Api Gateway]fix a bug that apiGateway start error May 27, 2020

Gzure changed the title ~~[Api Gateway]fix a bug that apiGateway start error~~ [Api Gateway] fix a bug that apiGateway start error May 27, 2020

Gzure changed the title ~~[Api Gateway] fix a bug that apiGateway start error~~ [Api Gateway] Fix a bug that apiGateway start error May 27, 2020

add WebFilter to handle keystone request and process project id

78385e3

xieus assigned Gzure May 27, 2020

xieus added bug Something isn't working feature feature development and removed bug Something isn't working labels May 27, 2020

xieus added this to the Version 0.5.2020.05.30 milestone May 27, 2020

xieus self-requested a review May 27, 2020 15:01

xieus reviewed May 27, 2020

View reviewed changes

...s/api_gateway/src/main/java/com/futurewei/alcor/apigateway/filter/KeystoneAuthWebFilter.java Outdated Show resolved Hide resolved

...s/api_gateway/src/main/java/com/futurewei/alcor/apigateway/filter/KeystoneAuthWebFilter.java Show resolved Hide resolved

xieus reviewed May 27, 2020

View reviewed changes

Gzure added 3 commits May 28, 2020 16:12

move neutron url prefix to application.properties

6599196

replace webclient to restclient to avoid webclient block exception

79a8690

add integration_keystone.adoc

01ad3c7

xieus reviewed May 29, 2020

View reviewed changes

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc Show resolved Hide resolved

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc Show resolved Hide resolved

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc Outdated Show resolved Hide resolved

Gzure added 7 commits June 1, 2020 11:26

add chache design in workflow and token design in integration_keyston…

95302ce

…e.adoc

add junit test for KeystoneAuthWebFilter

ccdb786

add cache in KeystoneAuthWebFilter

5a5473e

Update integration_keystone.adoc

8fcadd4

add log in KeystoneClient

7219fbe

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

aa03385

fix a unit test bug

8f6662d

xieus changed the title ~~[Api Gateway] Fix a bug that apiGateway start error~~ [Api Gateway] Support Authentication with OpenStack KeyStone Jun 3, 2020

Gzure added 2 commits June 3, 2020 17:16

remove some static class field define

2a8edc2

make doc pretty

d130457

xieus requested a review from cj-chung June 3, 2020 21:32

Gzure added 2 commits June 4, 2020 10:46

correct a spelling mistake

79410af

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

86ccba3

xieus reviewed Jun 4, 2020

View reviewed changes

lib/src/main/java/com/futurewei/alcor/common/entity/TokenEntity.java Outdated Show resolved Hide resolved

docs/modules/ROOT/pages/deploy_related/integration_keystone.adoc Outdated Show resolved Hide resolved

Gzure added 3 commits June 4, 2020 18:04

use @DaTa to replace setter and getter

238a127

add expire cache support

beee8b7

add test plan

27b0755

cj-chung approved these changes Jun 5, 2020

View reviewed changes

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

544d967

xieus reviewed Jun 5, 2020

View reviewed changes

Gzure added 7 commits June 5, 2020 18:43

add ICacheFactory interface and rewrite CacheFactory

fae5d02

add some method annotate

def42d0

add more test cases

a36fc07

rename 1.1 1.2 to 2.1 2.2

20754ff

rename utils.KeystoneClient to clients.KeystoneClient

d377031

add KeystoneClientTest for test KeystoneClient

fb70306

add KeystoneClientTest for test KeystoneClient

c34bac4

xieus approved these changes Jun 9, 2020

View reviewed changes

xieus added the security security feature and enhancement label Jun 9, 2020

xieus merged commit f5a47f7 into futurewei-cloud:master Jun 9, 2020

[Api Gateway] Support Authentication with OpenStack KeyStone #219

[Api Gateway] Support Authentication with OpenStack KeyStone #219

Uh oh!

Conversation

Gzure commented May 27, 2020

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Gzure commented May 29, 2020

Uh oh!

xieus commented May 29, 2020

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gzure commented May 30, 2020

Uh oh!

xieus commented May 30, 2020

Uh oh!

Gzure commented May 30, 2020

Uh oh!

Gzure commented Jun 3, 2020

Uh oh!

xieus commented Jun 3, 2020

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cj-chung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gzure commented Jun 5, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xieus commented Jun 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xieus commented Jun 5, 2020 •

edited

Loading