I would like to bring some change in the JIRA contrib project. The idea is to introduce a new public API for handling the Authentication part. Currently the JIRA extension have only one way to authenticate to a JIRA server. We can do it with Basic Authentication with the credential set on the Admin page. This can be really restrictive in some cases. For instance if I want that the macro displayed on the wiki don’t always show the same content depending of the user right on Jira.
A also created the main issue for this here: Loading...
I also thought about upgrading the HttpClient library before publishing this new API (to avoid a breaking change in the feature related to this). For this I created this ticket: Loading...
So what do you think about this ? Do you have an preference between the 2 solution of API ? And what do you think about the upgrade of the HttpClient lib ?
+1 for the general idea of making the authentication extensible.
I’ve tried to review the design proposal below.
We already have an Authentication interface in the form of a Component Role: HTTPJIRAFetcher. Currently this component is private but we could make it public (possibly with a more generic name). The current implementation is the one for the default hint and that should stay but any extension could provide a new component hint (like oauth for example).
We need a way to configure the JIRA extensions to know which authentication system to use: basic auth, oauth, etc. This should at least be available in xwiki.properties, and optionally in the Admin UI for the JIRA extension.
We need to make the current Admin UI of the JIRA extension configurable with an extension point, so that the UI displayed depends on the authentication system configured. For ex for Basic Auth, the user would enter the credentials. Note that the ability of configuring several JIRA URLs/ids should remain and be independent of the authentication system. For oauth, a different UI would be provided by an extension implementing oauth for JIRA.
On https://design.xwiki.org/xwiki/bin/view/Proposal/JiraauthenticationAPI you mentioned: For instance if I want that the macro displayed on the wiki don't always show the same content depending of the user right on Jira. AFAIU you want to tackle this by letting the user authenticate on the spot. Where would the credentials be saved for basic auth for ex? Could you should a flow diagram (or an explanation) of how this would work (not only for oauth but for any auth system)?
In JIRAServer you proposed to add a AuthenticationHandlerResult getAuthenticationHandler() method. Two problems: 1) there’s no AuthenticationHandler interface proposed on the design page and 2) the method name is not correct if it should return a Result and not a Handler.
I don’t like AuthenticationInfo maybeSetAuthentication() in JIRAServer as it seems very artificial. I don’t think we need to expose this. For me it’s an implementation detail to be done in the authenticator code.
Could you explain how AuthenticationState would be used? AFAIU you only need the info that authentication is needed. But how would the authenticator know that? It’s the jira server that knows that, and it depends on the issue. For example an issue with a Level = Confidential will require some specific rights but not other issues. Also if you take xwiki.org, you may be authenticated or you could also not be authenticated since the instance is public.
I’m not convinced by AuthenticationInfo and the fact that it makes the concept of link generic. For example for Basic Auth there’s no link, so the interface is not generic enough IMO. There are other types of auth that don’t work with a link either and that could work with some other mechanism (say a calculator or a code from a third party app, or …). So I’d keep this concept of “link” private to the authenticator implementation but also allow the HTTPJIRAFetcher to return something to display instead of just JIRA data. For example to generate some custom error message, or some link to authenticate, etc. One idea of implementation could be to throw a specific exception and then have the code using the HTTPJIRAFetcher decide how to handle it (by displaying the content of the Exception visually for example). Another option is to work on the return value (but it feels a bit more artificial).
Re Upgrade HttpClient to HttpClientv5, sure, this looks good, especially since it’s an internal change not affecting the users of the JIRA extension. Just make sure it works with XWiki 14.10.4 and above (since 14.10.4 is the minium version of XWiki currently supported by the JIRA Extension).
On the principle I’m not sure I fully agree with the jira macros displaying information not the same for all users (ie using different credentials). The goal of a wiki is to have a page containing content visible by lots of users and it’s better that they see the same content as much as possible (otherwise one user will say that he/she sees 10 issues while another no it’s wrong, there are 12 displayed - for ex). So I don’t disagree with the edge cases where we want to show content that depend on the user (like on some private wiki pages), but IMO the default should be credentials configured at the level of the Admin UI and for all users, whatever the auth system used.
Yes it’s also a possibility, I didn’t do this choice because I had the feeling that most of code might be identical in the other implementations. Maybe the solution might be to create an abstract class have a shared code for the code which is generic between the different HTTPJIRAFetcher implementations.
My idea was to play with the XWiki component for this. Currently we have the JIRAConfiguration interface which provide a list of JIRAServer. In case we add an other authentication system, we can just add an other extra implementation of this interface. When we need to get a JIRAServer object we can just iterate over the JIRAConfiguration instances to get all object of class JIRAServer.
Didn’t thought that we can extends the current configuration UI with an extension point. Seem a good idea to me as I agree we need a way to have a specific list of fields for each authentication system.
To clarify, the current issue with the service account is that when used JIRA will always return the result corresponding of the right of the service account. If the service account have the right to see everything on JIRA this could be an issue because we in some cases not all XWiki user have the right to see everything in JIRA.
So now the idea is to keep everything as it is for the basic auth, so we keep only one service account. But for a future implementation with OAuth we can request each user to proceed of the authentication to JIRA.
I agree that it’s a not easy question, after thinking again of the question I see theses different cases that can happen and what I expect that would happen:
User authenticated in XWiki
Type of authentication in JIRA extension
Expected comportment for public content
Expected comportment for private content
No
No authentication
Show
Hide
Yes
No authentication
Show
Hide
No
Basic auth
Show
Show what the service account can see
Yes
Basic auth
Show
Show what the service account can see
No
OAuth
Show
Hide and show a message that some content might be hidden
Yes
OAuth (without being authenticated to JIRA)
Show
Hide and show a message that some content might be hidden
Yes
OAuth (and the user is authenticated to JIRA)
Show
Show the content that the user can see
Following this, I’ll need to adapt the enum AuthenticationState to cover all cases.
Initially I thought also about the case when JIRA return an error like 401 which mean that the user must proceed of the authentication to see the content but I’m not sure that JIRA return this type of error.
For the OAuth case we can also think about 2 cases. For the private Jira instance we can decide to force the user to login before showing anything. And for the public instance as mentioned in the table we can show the public content with a message to say that the user need to be authenticated to have a better view of all Jira content.
I agree, it’s also why I proposed the option to return a list of block which is more generic. But note also that with a link you can redirect the everywhere. You can also redirect the user to an other page where the user will need to provide some additional info like code, …
The issue that I see to throw an exception is that we can’t return the result and in same time return an additional information, and this is a need, being able to return the data but also saying that the result is maybe not complete and to see more data the user need to proceed of the authentication.
For the point 5 and 6 I think I will need to review the design.
About this it would be good to have more details. I think it’s fine to have this feature in the contrib jira extension but we need to answer some details first.
Some questions:
I don’t think it’s related to the authentication system. What’s important is that there’s no authentication done and thus only public data is displayed
I think the message should only be displayed when source = JQL
Could you show the message you’d like to display and where it would be displayed depending on the style parameter used (table, list and enum)? IMO it’s going to be different locations for all styles, unless we use a tooltip on hover for example and in this case we can use the same solution for all styles.
Depending on the authentication system configured, the message could be different. For basic auth done in the Admin UI (ie same for all users), the message could include a link to the Admin UI section only if the current user has Admin rights. For oauth the message could have a link to authenticate for all users. Etc.
Point 4) shows that we need a component role that each auth system can implement to provide the message. Alternatively we could imagine something more generic where the result of the macro’s execute() method (ie list of blocks) could go through a Rendering Transformation to transform the XDOM into something a bit different. I feel that this is nice since it would provide a lot of power to an extension wanting to extend the JIRA macros.
Is the message only for the jira macro or also for other macros (count, graph, etc)? It feels like it should also be for these macros since the need is the same.
Not only because, even if you provide a list, depending of the right, the result could be limited depending of the right. So for instance if my list is XWIKI-1, XWIKi-2, XWIKI-3 and XWIKI-3 private, the macro result will just show XWIKI-1, XWIKi-2.
But yes I agree it will really depends of the style of the macro. And also if the macro is inline or not.
+1
I also think that having a more generic way to transform the XDOM after the macro execution seem a good idea. This is more generic and more flexible.
Yes we need this for the jiraChart, jira and jiraCount. And out of the macro context, we have also the jiraScriptService which will need to be able to handle OAuth, so we might have the same question… But for a scriptservice we can’t directly display a message.
ok, I was biased because I was thinking about the use case that everyone is supposed to see the same thing when viewing a page (ie the same list of jira issues) and if you enter the issues manually then you know you can see them and if you can see them then everyone can (since everyone uses the same auth).
But if you want to implement individual credentials then indeed it’s not limited to the JQL source.
I find the message quite intrusive on my side. It would be slightly less intrusive to be located at the bottom of the table. In any case if we implement item 5) with the Transformation(BTW I really mean using the existing Transformation component role) then it’s up to the authentication system to decide how to implement this. We would still need an implementation for basic auth in the jira contrib extension.
So in summary we just need to modify the jira macros in contrib to call a named Transformation (i.e. a Transformation with a specific hint), if it exists. Pretty simple.
