syndesisio / syndesis-project

Placeholder repository for project management
https://syndesis.io/
Apache License 2.0
6 stars 12 forks source link

Proposal for global settings section #28

Closed rhuss closed 7 years ago

rhuss commented 7 years ago

The design proposal should cover:

kahboom commented 7 years ago

I know we discussed this before, but I'm not 100% clear on whether or not this will be considered an administrative area and what user role would be required to access it?

rhuss commented 7 years ago

Actual we don't have different roles, so access to the admin area should be available without any condition.

dongniwang commented 7 years ago

Hi all, below is the first draft (early design) of the Global Settings Page.

The three rows in the design represent three different situations.

  1. For the Twitter one, nothing is configured. Users can click on the row to expand it and fill out the form. Client ID and client secret fields would be empty by default.
  2. For the Salesforce one, it's showing that users need to re-configure the client ID and client secret since there's something wrong with what they had provided previously.
  3. The last row with Facebook is showing the situation where everything is working properly.

I'm looking for feedback on the following:

I also have two questions:

Thank you very much! Look forward to hearing your feedback! Please tag appropriate reviewers here as well.

@rhuss @kahboom

global settings page v1 6 28

image

rhuss commented 7 years ago

@zregvart @jimmidyson as you are working on the OAuth flows you might be interested in a review of the global setup page, too.

zregvart commented 7 years ago

Is this a good place to discuss if we want to add support for the registration of OAuth applications in other systems?

A bit of background on terminology, registration means configuring Syndesis as an OAuth capable application with the OAuth resource provider (Twitter, Salesforce, Facebook, GitHub...) at that point a Client ID and a Client Secret is defined. Authorization is when the client gives access to its resources at the resource provider through OAuth flow. Registration is typically done out of band.

So for some resource providers we might have an automation in place, for others we might point to documentation or how-to videos (see screenshot for Salesforce). If we implement automation of registration do we do that from that link or do we need an action (for instance a button).

(see #12, #13)

zregvart commented 7 years ago

Is Remove All button really needed?

dongniwang commented 7 years ago

@zregvart: If I understand your comment about registration and authorization correctly, I think @sjcox-rh has provided an initial UX design for the Twitter OAuth flow (using the button approach). See #27

And about Remove All, it depends on if we want to support bulk action and how often do users want to do this. For instance, the Google My Account page does not have a Remove All functionality.

zregvart commented 7 years ago

@zregvart: If I understand your comment about registration and authorization correctly, I think @sjcox-rh has provided an initial UX design for the Twitter OAuth flow (using the button approach). See #27

I think that would be authorization. Registration would give us values for Client ID and Client Secret that we would input into this screen. Setting up a connector here first would be required step for authorization performed as described in #27.

And about Remove All, it depends on if we want to support bulk action and how often do users want to do this. For instance, the Google My Account page does not have a Remove All functionality.

This would disable every integration using those systems, not sure that users would want to do that in bulk.

zregvart commented 7 years ago

Oh, and let me answer the questions in the screenshot:

Is Connection and OAuth client management different from each other

I reason about this this way: Connection is per Syndesis user, and OAuth client management is per Syndesis platform. Or slightly more technical a Connection is an authorized OAuth client access.

Do users have multiple pairs of client ID and client secret for each connection? Or just one per connection?

I think we opted to have just one, and that would be one per Sydesis instance.

rhuss commented 7 years ago

@dongniwang thanks a lot, I like the overall, tab-based design for the global settings. I also think that a single tab is sufficient for managing the client registration as @zregvart describes.

First, we should be clear about whether we only want to allow a single client per type (e.g. a single Twitter client) or if we want to have multiple ones. I think a single one is sufficient for now.

As @zregvart mentioned a registration is required on this global settings page to connect the Syndesis installation with Twitter. Only then one or more Twitter connection can be created on top of this by doing the twitter specific Twitter OAuth workflow.

So we need to know what should happen if a user wants to create a Twitter connection in the global registration has not yet been performed:

Another question which I'm not completely sure about is, whether the required data is all the same for all OAuth types (Twitter/Salesforce/...) or whether we need to have fields per connector type. Is there any backend which allows an automatic acquisition of this data or is not always so that a user has to obtain Client ID and Secret manually from Twitter, Salesforce, .... ?


Also, I don't think that validation Client ID and Client Secret is easily possible without a concrete connections. In a first step, I would only check, whether such data is provided but dont validate it. This would happen automatically when a connection is created (with the appropriate error message pointing back to the Client ID and Secrete if this is wrong). @kcbabo would be acceptable to ommit the validatio of the client id and secret at that point ?

@dongniwang for the UI this would imply that we can't offer the status for "working properly" or "given, but broken". We can only have the status configure / not configured.

kcbabo commented 7 years ago

If client ID/secret can only be validated in combination with a connection, we should just make sure we document that for users. Perhaps even include some text to that effect in the UI itself (e.g. "You can validate these values when creating a connection for Twitter").

gashcrumb commented 7 years ago

So we need to know what should happen if a user wants to create a Twitter connection in the global registration has not yet been performed:

Should the "Create Twitter Connection" be disabled ? Should there be a link to the global settings page from the Twitter connection based for entering registration ? Should it be automatically a dialog for the client id / creds on the connection base before initializing the OAuth flow, when no registration has been performed ?

Kinda seems like just adding an interim step that prompts the user for these things if required would be good. I wouldn't make it a dialog, and I don't think flipping them over to the global settings page would be good either. Rather it feels like just prompting for the settings either an interim page or on the same page as the connector configuration page would be better.

One general question on this page, how many external apps will we be supporting eventually? Enough to warrant a search/filter field? Even 5-8 external apps means I'd have to scroll through the page and use my lookin' eyes to find the right one.

Another question, if I click remove does that hose all integrations using those credentials immediately?

dongniwang commented 7 years ago

@rhuss @kcbabo Thanks for the comments!

global settings page

dongniwang commented 7 years ago

Kinda seems like just adding an interim step that prompts the user for these things if required would be good. I wouldn't make it a dialog, and I don't think flipping them over to the global settings page would be good either. Rather it feels like just prompting for the settings either an interim page or on the same page as the connector configuration page would be better.

Agreed! Adding @sjcox-rh here for some feedback as well.

One general question on this page, how many external apps will we be supporting eventually? Enough to warrant a search/filter field? Even 5-8 external apps means I'd have to scroll through the page and use my lookin' eyes to find the right one.

If we have more than what would eventually go below the fold, I think we should have filter and search.

gashcrumb commented 7 years ago

Oh one other thing I had a question about and maybe this has already been discussed, is there ever a reason that someone might want to use more than one clientId/secret for an external app? Like a cheap test instance vs. the really expensive production account that has a higher request quota?

rhuss commented 7 years ago

@gashcrumb very good question, I would say 'no' for the first shot, but @kcbabo has the real answer :)

rhuss commented 7 years ago

Would it make sense to include the registration process in the Create a Connection workflow? If registration is already configured by admin (in some cases), just show a form with client ID & client secret listed but not editable.

I wonder whether we could make it optional to show this step if the data has been already provided.

Actually I also wonder a bit about the possible roles: The registration is done by the Syndesis "admin" (or "expert") role, the authorization for the connection is done by the "citizen developer" role. So it might be that the citizen developer is not allowed to register the application or even see the secret here. But maybe I think over the top here, so we might not worry about different authorization for different roles at the moment.

My understanding is that required data is not the same for all the OAuth types. Please correct me if I'm wrong.

I'm not 100% sure, but afaik the OAuth standard doesn't not asses the required parameters, so there could be more then client id and secret. @zregvart knows this for sure ;)

If we can't show the status directly in the UI, I agree we can at least show some text to inform users.

Still not 100% sure whether a given client id and secret can be verified or not with a request to the backend, but I'm pretty sure even if this is possible for some backend, its not possible for all backends.

zregvart commented 7 years ago

My understanding is that required data is not the same for all the OAuth types. Please correct me if I'm wrong.

I'm not 100% sure, but afaik the OAuth standard doesn't not asses the required parameters, so there could be more then client id and secret. @zregvart knows this for sure ;)

As far as I understand, we could get by with just client id and client secret, and as I gather @chirino believes we should make this as simple as possible and use only those. Salesforce in particular supports more than this, for instance using Certificates and then for authorization using JWT OAuth flow that is nice as it never times out (no need for refresh tokens).

chirino commented 7 years ago

@dongniwang I like it. We should perhaps also make room to embed a how to register the app video similar to: https://youtu.be/_EariY0sJ5U

@zregvart yes, let's keep it as simple as possible for now. If/when we run into a system that needs multiple client id's (i could imagine for something like a Dev/prod split) we can figure out how to handle it then.

kcbabo commented 7 years ago

@rhuss @gashcrumb My understanding of of client ID/secret is that it identifies the iPaaS application as the client. In your example, the user would likely have a test iPaaS instance that corresponds to the cheap test account and a production iPaaS instance for the prod account. If that holds true, then a single client ID/secret pair would suffice.

rhuss commented 7 years ago

@kcbabo that works if its against the same backend (e.g. Twitter and GitHub fall in this category). But what if there are different installations of the same application, each with its own security management ? Then ipaas would has to identify as client to both systems. Not sure about Salesforce (probably not). If this is completely made up, I'm happy with having a single registration per type. And even not, I'm still happen to restrict ourselves to a single registration ;)

kcbabo commented 7 years ago

How do you feel about starting with one registration per connector and then letting requirements / feedback guide if/how multiple registrations would work?

rhuss commented 7 years ago

@kcbabo I'm perfectly fine with that :)

rhuss commented 7 years ago

I think now that we all have an idea for how the global settings section should look like, we can start the proposal (in the same spirit as the other ones).

Who wants to pick up this task? @iocanel @KurtStam @zregvart @kahboom any cycles for this ? I'm currently blocked with the lifecycle backend changes, retro action items, and project management. Any other volunteers?

rhuss commented 7 years ago

Design proposal has been merged (although there were some questions open).

However, we are going to close this issue here, too. If we need further clarification for the global settings page, we need to create a new issue (or reopen this one).