Sync auth by wtrocki · Pull Request #403 · aerogear/mobile-docs

wtrocki · 2018-08-30T10:02:51Z

Posting for early visibility.

wtrocki · 2018-08-31T13:59:25Z

modules/ROOT/pages/sync/auth.adoc

+[id='{context}_authentication']
+= Authentication
+
+The {sync-service} by default do not provide any any authentication and authorization mechanism.


@psturc Actually tried to execute this steps and see how long this takes. It's not simple or convenient which made me question entire instruction. pinged @ziccardi to see if we can just get config directly from keycloak server and paste that directly as content to environment variable. If this is working we can document that - possibly instruction will change to 3 simple steps, but it will not include binding.

that sounds good

This is going to be separate ticket as @ziccardi is out sick.

wtrocki · 2018-09-04T08:18:09Z

Can we check this again? It's still pretty raw (we can improve it later).
If we merge that we going to unblock @darahayes PR that may be related to this.

darahayes

Hey @wtrocki apologies for the very specific review. It's all minor grammar things but they're important to get right.

darahayes · 2018-09-04T08:28:44Z

modules/ROOT/pages/sync/auth.adoc

+[id='{context}_authentication']
+= Authentication
+
+The {sync-service} by default do not provide any any authentication and authorization mechanism.


Perhaps rework this to the following:

By default, {sync-service} does not provide any authentication and authorization mechanism.

+1 to Dara's rewording

darahayes · 2018-09-04T08:30:59Z

modules/ROOT/pages/sync/auth.adoc

 * Ensure {keycloak-service} is xref:keycloak/provisioning.adoc[provisioned]
+* Ensure {keycloak-service} is xref:keycloak/coding.adoc[configured] to work with your mobile applications
+
+== Binding the {sync-service} service to a {keycloak-service} instance


Best practice is to captialize headings, except for words like is, a, the, to etc.

Should be:

Binding the {sync-service} Instance to a {keycloak-service} Instance.

fyi, there's a debate about that style issue, Dara describes the style called Titlecase, more likely we're going with Sentence case, which is what is there currently

darahayes · 2018-09-04T08:32:01Z

modules/ROOT/pages/sync/auth.adoc

+
+== Binding the {sync-service} service to a {keycloak-service} instance
+
+To benefit from authentication developers need to connect (bind) {sync-service} into  {keycloak-service} instance. 


Minor grammar thing:

To benefit from authentication, developers...

To implement authentication in your app, you must bind the {sync-service} service with the {keycloak-service} service.

darahayes · 2018-09-04T08:33:15Z

modules/ROOT/pages/sync/auth.adoc


 NOTE: The mount path must be set to `/opt/keycloak` for the {sync-service} to be configured to use the secret.

+== Making secret available


Again, capitalization. Should probably be:

Making the Secret Available

darahayes · 2018-09-04T08:34:38Z

modules/ROOT/pages/sync/auth.adoc


+== Making secret available
+
+By default secret is not visible to server application


Minor grammar thing. Should probably be like this:

By default, the secret is not visible to the server application

darahayes · 2018-09-04T08:36:23Z

modules/ROOT/pages/sync/auth.adoc

+== Making secret available
+
+By default secret is not visible to server application
+To point server to application please add new environment variable:


Again just some small grammar stuff,

To point the server to the application, please add a new environment variable:

jhellar · 2018-09-04T11:00:24Z

The Authentication section is now in Setting up section. Shouldn't it be in it's own section similarly to Monitoring Data Sync Service section?

jhellar · 2018-09-04T11:27:00Z

After completing the steps, and opening data sync server url on /graphql endpoint, I get Invalid parameter: redirect_uri. Should we also add steps for configuring valid redirect uri?

jhellar · 2018-09-04T11:38:59Z

Will this work with self-signed certs? Should we document it?

psturc · 2018-09-04T15:56:51Z

The trick for getting this working with self-signed certs is to add NODE_TLS_REJECT_UNAUTHORIZED = 0 to environment variables in the data-sync-server deployment config. This should be documented.

pwright · 2018-09-05T09:29:13Z

modules/ROOT/pages/sync/adding-sync-to-your-app.adoc

 include::{partialsdir}/attributes.adoc[]

-= Using the {sync-service} service SDK
+= Using the {sync-service} SDK


not the way we refer to SDK in doc, but will fix in post

Ah.. Really sorry. Missing tons of context here.

wtrocki · 2018-09-05T09:31:47Z

Let's create another ticket to review entire auth docs:

The Authentication section is now in Setting up section. Shouldn't it be in it's own section similarly to Monitoring Data Sync Service section?

After completing the steps, and opening data sync server url on /graphql endpoint, I get Invalid parameter: redirect_uri. Should we also add steps for configuring valid redirect uri?

This is part of the prerequisites (Keycloak setup and running) instruction there explains it and I did not want to copy the content here.

Will this work with self-signed certs? Should we document it?

This is canonical documentation ( we will need to get some local machine documentation that will cover self signed certs.

wtrocki · 2018-09-05T09:32:45Z

@jhellar @psturc Can you take look and get that very raw version of this. Can we get some consensu how to reorganize this content better, basing on your valuable feedback.

wtrocki · 2018-09-05T10:00:50Z

@darahayes Final review?

wtrocki · 2018-09-05T10:39:16Z

Mering to sync branch in order to rebuild, joing articles together

StephenCoady · 2018-09-05T10:41:09Z

modules/ROOT/pages/sync/auth.adoc

+o point the server to the application, please add a new environment variable:
+
+----
+KEYCLOAK_CONFIG_FILE= `/opt/keycloak`TODO


are we leaving this as is? we could change it to the actual value we are using for now?

Ups. yeah. TODO needs to be removed. I overlooked that in the rush

StephenCoady · 2018-09-05T10:58:37Z

Its ok I will change it on sync branch. I was too slow reviewing.

…

On Wed, 5 Sep 2018 at 11:49, Wojciech Trocki ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In modules/ROOT/pages/sync/auth.adoc <#403 (comment)>: > + +== Making the Secret available to the {sync-service} + +* Select *View Secret* on the *Identity Management* details page +* Select *Add to Application* and select the data-sync-server from the options +* Select the *Volume* option and enter the following mount path: `/opt/keycloak` + +NOTE: The mount path must be set to `/opt/keycloak` for the {sync-service} to be configured to use the secret. + +== Making Secret available to application + +By default, the secret is not visible to the server application. +o point the server to the application, please add a new environment variable: + +---- +KEYCLOAK_CONFIG_FILE= `/opt/keycloak`TODO Ups. yeah. TODO needs to be removed. I overlooked that in the rush — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#403 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG9_8-PQa1uD6HAUruIknIyciGs68IgSks5uX6w4gaJpZM4WTKnE> .

-- STEPHEN COADY ASSOCIATE SOFTWARE ENGINEER Red Hat <https://www.redhat.com/> Communications House, Cork Road Waterford City, Ireland X91NY33 scoady@redhat.com IM: scoady <https://red.ht/sig>

wtrocki requested review from jhellar and psturc August 30, 2018 10:02

wtrocki requested review from jstaffor and pwright as code owners August 30, 2018 10:02

wtrocki changed the base branch from master to sync August 30, 2018 10:19

wtrocki requested a review from darahayes August 30, 2018 10:19

pwright approved these changes Aug 30, 2018

View reviewed changes

wtrocki mentioned this pull request Aug 31, 2018

Sync authorization docs #416

Merged

wtrocki commented Aug 31, 2018

View reviewed changes

darahayes reviewed Sep 4, 2018

View reviewed changes

pwright reviewed Sep 5, 2018

View reviewed changes

wtrocki force-pushed the sync-auth branch 2 times, most recently from 7f4b5ae to a3fc127 Compare September 5, 2018 09:58

wtrocki and others added 5 commits September 5, 2018 10:59

Initial version of Sync auth docs

a5d09aa

WIP

14feb4b

Update auth.adoc

9473609

Fix - data-sync-server option

8091a77

Review comments addressed

baa57a5

wtrocki force-pushed the sync-auth branch from a3fc127 to baa57a5 Compare September 5, 2018 09:59

wtrocki merged commit e606ce2 into sync Sep 5, 2018

wtrocki deleted the sync-auth branch September 5, 2018 10:39

StephenCoady reviewed Sep 5, 2018

View reviewed changes


		== Binding the {sync-service} service to a {keycloak-service} instance

		To benefit from authentication developers need to connect (bind) {sync-service} into {keycloak-service} instance.


		NOTE: The mount path must be set to `/opt/keycloak` for the {sync-service} to be configured to use the secret.

		== Making secret available


		== Making secret available

		By default secret is not visible to server application

Conversation

wtrocki commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtrocki Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

wtrocki commented Sep 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darahayes 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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

jhellar commented Sep 4, 2018

Uh oh!

jhellar commented Sep 4, 2018

Uh oh!

jhellar commented Sep 4, 2018

Uh oh!

psturc commented Sep 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtrocki commented Sep 5, 2018

Uh oh!

wtrocki commented Sep 5, 2018

Uh oh!

wtrocki commented Sep 5, 2018

Uh oh!

wtrocki commented Sep 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StephenCoady commented Sep 5, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wtrocki commented Aug 30, 2018 •

edited

Loading

wtrocki Aug 31, 2018 •

edited

Loading

wtrocki commented Sep 4, 2018 •

edited

Loading