Skip to content
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.

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

[Proposal] syntactic sugar for JIRA::HTTPError due to Atlassian basic auth changes #336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

salguodnz
Copy link

Hi all,

This is a proposal as to how this gem can be more consumer friendly. Currently, there are a large number of issues due to people being tripped up by the change to Atlassian auth changes. (i.e. not a user password anymore, but actually an access token).

I've observed a number of these JIRA::HTTPError related issues raised against jira-ruby.
I was also caught in the same problem. I did see that there was some documentation added - but personally I don't think many people read it super closely.

My proposal is this: for basic auth, let's make it explicit that you need an api_access_token, and for cookie based auth - its still the password parameter. Whether the change to the basic caused actual integration problems or not - people are getting caught out by this 'gotcha'.

Please bear with me - I am not a ruby developer - I have had a lot of time in a fastlane environment but by trade I am not a ruby developer. I would be making the corresponding change there too.
I am not sure on versioning but this would be a breaking change for most of the consumers of this gem.

If this proposal was accepted - if possible - I'd want someone to verify the spec changes I made.
When I did it locally, I either got a bunch of HTTPMock errors about not actually making an HTTP call - with or without jira running.

Thanks!

@p-mongo
Copy link

p-mongo commented Jul 15, 2019

The basic auth parameters are a username and a password. It is strange to have a username/api access token, not to mention this is a breaking change for applications (changing from password to token value requires reconfiguration only, but this PR would require code changes).

Instead I would suggest that improving diagnostic messaging would likely be more helpful. Specifically, if a request fails with 401 (which is what I assume happens) add to the exception message a note like "if using basic authentication, you may need to pass an API token rather than your plain text password".

@salguodnz
Copy link
Author

Yes, I agree. Basic Auth by standard is username/password driven. And yes, this would be a breaking change. But that breaking change has already happened downstream when Atlassian changed their API Auth - and is manifesting in Jira-ruby because the parameter naming isn’t intuitive. Look at the number of issues in this repo - a decent amount are purely due to this. The maintainers can either keep commenting on every one of them and link the Atlassian API changes, or we can better the user experience and ease-of-use of the Jira-ruby gem and call things what they are. We are unfortunately tied to Atlassians standard, not basic Auth.
If this chain of thought isn’t sitting well with the maintainers, I’ll close this and reopen another PR with some diagnostic messaging at the very least.

@p-mongo
Copy link

p-mongo commented Jul 15, 2019

The link mentioned in this PR, https://confluence.atlassian.com/cloud/api-tokens-938839638.html, talks about Atlassian Cloud. We have a hosted Jira installation and up to about a week ago were successfully using basic auth. While I imagine the Atlassian's jira accounts for most usage, this particular PR may seriously confuse users of hosted Jira installations.

@p-mongo
Copy link

p-mongo commented Jul 15, 2019

To your point of users of jira-ruby being confused, I am one of them and I ended up directly calling the REST API because, quite possibly, I couldn't figure out how to make jira-ruby authenticate. I am now attempting this again with oauth and it's not really any clearer. The error diagnostics provided by this library can be improved a good amount (#339).

@salguodnz
Copy link
Author

salguodnz commented Jul 15, 2019

Fair points.
I had the same points of confusion using this gem.
I just am struggling with the fact that while diagnostic messages could help, we'd still a sense condemning people to confusion - in which they may create issues against this repo, double check their plaintext passwords (as the parameter is called password), reset their Jira passwords, dig through google/ SO / this repo only to find that password for basic auth means api access token.

Is there some compromise that circumvents this needless confusion?
Highlighting it in the readme might help, but for me that’s a plan D. I feel that adding comments against sample code isn’t really the best solution - “comments are a failure to communicate intent”.
No interpreter interprets it logically, nor compiler compiles it, nor tests cover the comments intent.

@garciaf
Copy link
Collaborator

garciaf commented Jul 16, 2019

I actually like this approach,
I think it is valuable to distinguish further Jira server and Jira cloud.
Because they have very different way of authentication, and the code reflecting this make it explicit.

It will need to be part of a Major release version because it will definitely break existing implementation.
But I really like it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants