Add support for Couchbase's role based access #16389

enesacikoglu · 2019-03-29T15:09:52Z

Couchbase role-based access for bucket released after version 5.X.But ,there is no configuration support for Spring Data Couchbase auto configuration after couchbase version 5.X takes username for authentication.

Related problem as linked below;

https://stackoverflow.com/questions/47169384/couchbase-bucket-authentication-error

So, I added username to couchbase properties to solve problem.

pivotal-issuemaster · 2019-03-29T15:09:54Z

@enesacikoglu Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster · 2019-03-29T15:11:12Z

@enesacikoglu Thank you for signing the Contributor License Agreement!

…e client

snicoll · 2019-04-01T08:21:39Z

@bsubhashni wondering if you have a bit of time to look at this one.

bsubhashni · 2019-04-02T16:18:44Z

Adding @dnault

enesacikoglu · 2019-04-06T08:31:32Z

Hello,

Waiting for review.

Thanks

snicoll · 2019-04-06T16:04:38Z

@enesacikoglu please be patient, I've already triaged your PR and asked for some feedback from the Couchbase team.

dnault

@enesacikoglu Thank you. Support for Role Based Access Control (RBAC) is a much needed feature. Here are some notes.

dnault · 2019-04-08T19:43:55Z

...gure/src/main/java/org/springframework/boot/autoconfigure/couchbase/CouchbaseProperties.java

+		/**
+		 * RoleBaseAccessEnable for support Couchbase bucket after version 5.0.
+		 */
+		private boolean roleBaseAccessEnabled = false;


Rather than requiring the user to explicitly enable RBAC, RABC should automtatically be enabled when the username is set.

removed roleBaseAccessEnabled properties.

dnault · 2019-04-08T19:45:14Z

...gure/src/main/java/org/springframework/boot/autoconfigure/couchbase/CouchbaseProperties.java

+		/**
+		 * Username of the bucket.
+		 */
+		private String userName = "";


RBAC credentials are for the whole cluster, not just one bucket. Instead of adding the username to the bucket, please add username and password as top level properties of CouchbaseProperties. Note the capitalization; "username" is one word in this context, and should be all lowercase.

Please keep the separate bucket password for backwards compatibility with non-RBAC clusters.

moved username and password to top level of properties.

dnault · 2019-04-08T19:51:33Z

...e/src/main/java/org/springframework/boot/autoconfigure/couchbase/CouchbaseConfiguration.java

+		CouchbaseProperties.Bucket bucket = this.properties.getBucket();
+		if (bucket.isRoleBaseAccessEnabled()) {
+			return couchbaseCluster.authenticate(bucket.getUserName(),
+					bucket.getPassword());


Also need to change CouchbaseConfiguration.couchbaseClient() to call openBucket(bucket.name) if RBAC is enabled or openBucket(bucket.name, bucket.password) if RBAC is disabled. Otherwise the client will throw MixedAuthenticationException.

Right thanks :)
added also here with RBAC control.

enesacikoglu · 2019-04-09T10:53:04Z

@enesacikoglu Thank you. Support for Role Based Access Control (RBAC) is a much needed feature. Here are some notes.

@dnault thanks for your kindly review and comments.
I try to implement new feature via your suggestions.

Could you review again ?

dnault

Thanks for updating the PR. One more comment:

dnault · 2019-04-09T17:10:17Z

...e/src/main/java/org/springframework/boot/autoconfigure/couchbase/CouchbaseConfiguration.java

+		CouchbaseCluster couchbaseCluster = CouchbaseCluster
+				.create(couchbaseEnvironment(), determineBootstrapHosts());
+		if (this.properties.getUsername() != null
+				&& this.properties.getPassword() != null) {


The username and password properties are initialized to empty string (""). Will they ever be null? A better check might be .isEmpty().

Opps how can ı miss 👎 Thanks for your awesome attention

dnault · 2019-04-10T09:14:07Z

...e/src/main/java/org/springframework/boot/autoconfigure/couchbase/CouchbaseConfiguration.java

@@ -79,6 +86,10 @@ public ClusterInfo couchbaseClusterInfo() {
 	@Bean
 	@Primary
 	public Bucket couchbaseClient() {
+		if (this.properties.getUsername().isEmpty()
+				|| this.properties.getPassword().isEmpty()) {
+			return couchbaseCluster().openBucket(this.properties.getBucket().getName());


This condition should be inverted. If RBAC username or password are empty we should call the openBucket method that takes a bucket password.

Sorry if I missed this earlier.

This condition should be inverted. If RBAC username or password are empty we should call the openBucket method that takes a bucket password.

Sorry if I missed this earlier.

Thanks. I inverted it. 👍

dnault

@enesacikoglu Thank you, nice work.

@snicoll Couchbase approves the code change. Is there documentation that needs to be updated?

See gh-16389

* pr/16389: Polish "Add support for Couchbase's role-based access" Add support for Couchbase's role-based access

snicoll · 2019-05-21T07:19:16Z

@enesacikoglu thanks for making your first contribution to Spring Boot. This is now merged with a polish commit.

In particular, I didn't like that username and password are set to blank and that the condition that checks if RBA is enabled wasn't shared. There are now null and we check they are both set, as this article states that:

You cannot create a user without a password.

snicoll · 2019-05-21T07:20:19Z

Support for Role Based Access Control (RBAC) is a much needed feature.

@dnault in the future, please feel free to reach out to us if there are changes in Couchbase that require a change on our side. Thanks!

sadikkuzu · 2019-05-25T20:11:04Z

Congrats @enesacikoglu
You're doing the best!

spring-projects#16388 couchbase 5.x and later auth

7f81cc1

snicoll mentioned this pull request Mar 29, 2019

Username Support for 5.x and later versions of Couchbase to Spring Data Couchbase #16388

Closed

snicoll changed the title ~~#16388 couchbase 5.x and later auth~~ Support for role-based access for Couchbase bucket Mar 29, 2019

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 29, 2019

Enes Açıkoğlu added 3 commits March 29, 2019 18:18

spring-projects#16388 javadoc added

91c798b

spring-projects#16388 checkstyle fixs

67270fd

spring-projects#16388 rolebaseacces properties added for new couchbas…

74f8f57

…e client

dnault suggested changes Apr 8, 2019

View reviewed changes

Enes Açıkoğlu added 2 commits April 9, 2019 10:39

spring-projects#16388 review fixs

e0aeb65

Merge remote-tracking branch 'upstream/master' into feature/16388

e772ad0

dnault suggested changes Apr 9, 2019

View reviewed changes

spring-projects#16388 null check fix

f098f02

dnault suggested changes Apr 10, 2019

View reviewed changes

Enes Açıkoğlu added 2 commits April 10, 2019 13:04

spring-projects#16388 fix openbucket on RBAC

38df10b

Merge remote-tracking branch 'upstream/master' into feature/16388

8e19ebd

dnault approved these changes Apr 10, 2019

View reviewed changes

snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels May 15, 2019

snicoll added this to the 2.2.x milestone May 15, 2019

snicoll self-assigned this May 15, 2019

snicoll changed the title ~~Support for role-based access for Couchbase bucket~~ Add support for Couchbase's role based access May 21, 2019

snicoll modified the milestones: 2.2.x, 2.2.0.M4 May 21, 2019

snicoll pushed a commit that referenced this pull request May 21, 2019

Add support for Couchbase's role-based access

3386856

See gh-16389

snicoll closed this in 2949561 May 21, 2019

snicoll added a commit that referenced this pull request May 21, 2019

Merge pull request #16389 from enesacikoglu

9b73d65

* pr/16389: Polish "Add support for Couchbase's role-based access" Add support for Couchbase's role-based access

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Couchbase's role based access #16389

Add support for Couchbase's role based access #16389

enesacikoglu commented Mar 29, 2019

pivotal-issuemaster commented Mar 29, 2019

pivotal-issuemaster commented Mar 29, 2019

snicoll commented Apr 1, 2019

bsubhashni commented Apr 2, 2019

enesacikoglu commented Apr 6, 2019

snicoll commented Apr 6, 2019

dnault left a comment

dnault Apr 8, 2019

enesacikoglu Apr 9, 2019

dnault Apr 8, 2019

enesacikoglu Apr 9, 2019

dnault Apr 8, 2019

enesacikoglu Apr 9, 2019

enesacikoglu commented Apr 9, 2019

dnault left a comment

dnault Apr 9, 2019

enesacikoglu Apr 9, 2019

dnault Apr 10, 2019

enesacikoglu Apr 10, 2019

dnault left a comment

snicoll commented May 21, 2019

snicoll commented May 21, 2019

sadikkuzu commented May 25, 2019

Add support for Couchbase's role based access #16389

Add support for Couchbase's role based access #16389

Conversation

enesacikoglu commented Mar 29, 2019

pivotal-issuemaster commented Mar 29, 2019

pivotal-issuemaster commented Mar 29, 2019

snicoll commented Apr 1, 2019

bsubhashni commented Apr 2, 2019

enesacikoglu commented Apr 6, 2019

snicoll commented Apr 6, 2019

dnault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enesacikoglu commented Apr 9, 2019

dnault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnault left a comment

Choose a reason for hiding this comment

snicoll commented May 21, 2019

snicoll commented May 21, 2019

sadikkuzu commented May 25, 2019