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.

Sign up for GitHub

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

Jump to bottom

Support for Basic Authentication #492

Merged

majst01 merged 1 commit into influxdata:master from bonitoo-io:basic_auth

Aug 22, 2018

Contributor

lxhoan commented Aug 21, 2018

No description provided.

codecov-io commented Aug 21, 2018 •

edited

Loading

Codecov Report

Merging #492 into master will increase coverage by 0.04%.
The diff coverage is 93.75%.

@@             Coverage Diff             @@
##             master    #492      +/-   ##
===========================================
+ Coverage     87.35%   87.4%   +0.04%     
- Complexity      362     364       +2     
===========================================
  Files            24      25       +1     
  Lines          1503    1500       -3     
  Branches        167     167              
===========================================
- Hits           1313    1311       -2     
+ Misses          124     123       -1     
  Partials         66      66

Impacted Files	Coverage Δ	Complexity Δ
...n/java/org/influxdb/impl/BasicAuthInterceptor.java	`100% <100%> (ø)`	`2 <2> (?)`
src/main/java/org/influxdb/impl/InfluxDBImpl.java	`89.8% <90%> (+0.02%)`	`78 <4> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f31b2fd...090864f. Read the comment docs.

Collaborator

majst01 commented Aug 21, 2018

LGTM

Collaborator

majst01 commented Aug 21, 2018

Hi @lxhoan would you mind adding a CHANGELOG entry ?

lxhoan changed the title ~~imlement issue #449 : support for Basic Authentication~~ Support for Basic Authentication


          imlement issue influxdata#449 : support for Basic Authentication

090864f

lxhoan force-pushed the basic_auth branch from cb00d77 to 090864f Compare

August 21, 2018 10:15

Contributor Author

lxhoan commented Aug 21, 2018

changelog added

fmachado reviewed

View reviewed changes

Contributor

fmachado left a comment

With this PR, we will create a new minor version that is not backward compatible. Is this correct or I am missing something?

Contributor Author

lxhoan commented Aug 21, 2018

with reference to InfluxDb document

https://docs.influxdata.com/influxdb/v1.6/administration/authentication_and_authorization/
Hi @fmachado, it's still backward compatible (actually in the client now Authentication scheme is transparent to user). I just propose to to switch to Basic Auth because in InfluxDB document

https://docs.influxdata.com/influxdb/v1.6/administration/authentication_and_authorization/

basic authentication "is the preferred method for providing user credentials"

Contributor

fmachado commented Aug 21, 2018

@lxhoan ok but you modified the existing public API by removing parameters and this will break the existing users implementation when they update the library.

fmachado reviewed

View reviewed changes

src/main/java/org/influxdb/impl/InfluxDBService.java

@@ @@ -39,49 +39,44 @@ @@
                  *                             Can be one of one, any, all, quorum. Defaults to all.
                  */
                 @POST("write")
-                public Call<ResponseBody> writePoints(@Query(U) String username,
-                    @Query(P) String password, @Query(DB) String database,
+                public Call<ResponseBody> writePoints(@Query(DB) String database,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                     @Query(RP) String retentionPolicy, @Query(PRECISION) String precision,
                     @Query(CONSISTENCY) String consistency, @Body RequestBody batchPoints);
                 @GET("query")
-                public Call<QueryResult> query(@Query(U) String username, @Query(P) String password, @Query(DB) String db,
+                public Call<QueryResult> query(@Query(DB) String db,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                     @Query(EPOCH) String epoch, @Query(value = Q, encoded = true) String query);
                 @POST("query")
-                public Call<QueryResult> query(@Query(U) String username, @Query(P) String password, @Query(DB) String db,
+                public Call<QueryResult> query(@Query(DB) String db,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                         @Query(EPOCH) String epoch, @Query(value = Q, encoded = true) String query,
                         @Query(value = PARAMS, encoded = true) String params);
                 @GET("query")
-                public Call<QueryResult> query(@Query(U) String username, @Query(P) String password, @Query(DB) String db,
+                public Call<QueryResult> query(@Query(DB) String db,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                     @Query(value = Q, encoded = true) String query);
                 @POST("query")
-                public Call<QueryResult> postQuery(@Query(U) String username, @Query(P) String password, @Query(DB) String db,
+                public Call<QueryResult> postQuery(@Query(DB) String db,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                     @Query(value = Q, encoded = true) String query);
                 @POST("query")
-                public Call<QueryResult> postQuery(@Query(U) String username, @Query(P) String password, @Query(DB) String db,
+                public Call<QueryResult> postQuery(@Query(DB) String db,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                         @Query(value = Q, encoded = true) String query, @Query(value = PARAMS, encoded = true) String params);
                 @GET("query")
-                public Call<QueryResult> query(@Query(U) String username, @Query(P) String password,
-                    @Query(value = Q, encoded = true) String query);
+                public Call<QueryResult> query(@Query(value = Q, encoded = true) String query);

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                 @POST("query")
-                public Call<QueryResult> postQuery(@Query(U) String username,
-                    @Query(P) String password, @Query(value = Q, encoded = true) String query);
+                public Call<QueryResult> postQuery(@Query(value = Q, encoded = true) String query);

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                 @Streaming
                 @GET("query?chunked=true")
-                public Call<ResponseBody> query(@Query(U) String username,
-                    @Query(P) String password, @Query(DB) String db, @Query(value = Q, encoded = true) String query,
+                public Call<ResponseBody> query(@Query(DB) String db, @Query(value = Q, encoded = true) String query,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

src/main/java/org/influxdb/impl/InfluxDBService.java

                     @Query(CHUNK_SIZE) int chunkSize);
                 @Streaming
                 @POST("query?chunked=true")
-                public Call<ResponseBody> query(@Query(U) String username,
-                        @Query(P) String password, @Query(DB) String db, @Query(value = Q, encoded = true) String query,
+                public Call<ResponseBody> query(@Query(DB) String db, @Query(value = Q, encoded = true) String query,

Contributor

fmachado Aug 21, 2018

parameters removed: (non-backward compatible change)

Collaborator

majst01 Aug 21, 2018

No, InfluxDBService provides only a internal API which @lxhoan adopted, the only visible API is InfluxDB,java and that didnt change.

Or did i miss something ?

Contributor Author

lxhoan Aug 22, 2018 •

edited

Loading

the method signatures are all public however the interface (InfluxDBService) itself is visible to org.influxdb.impl only, so I don't think user can import this interface in their implementation

fmachado approved these changes

View reviewed changes

Contributor

fmachado left a comment

Ops! I missed the class access modifier!

majst01 merged commit 24c5542 into influxdata:master

lxhoan added this to the 2.13 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment