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

ISSUE-232 feature flag addition of the oauth_token parameter #378

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

Conversation

davidalpert
Copy link
Contributor

@davidalpert davidalpert commented Jan 23, 2021

this PR should help address #232

previously an empty oauth_token parameter was appended to the
query before sending it to jira when the :auth_type parameter
was set to :oauth_2legged, but it appears that oauth v0.5.5
is appending the actual value of oauth_token as a parameter

as a result the code which was appending an empty oauth_token
parameter was actually appending it to the existing oauth_token
value, thereby corrupting the token and resulting in a vague

parameter_rejected (OAuth::Problem)

error. I had to look at the detail in the server
atlassian-jira.log file in order to see the actual problem.

in order to preserve behaviour and backwards compatibilty with
the previous release of the jira-ruby gem, this commit introduces
a new feature flag :append_explicit_oauth_token_parameter which
defaults to true, same as before, but can now be explicitly
toggled off to avoid corrupting the access token in this way.

using this commit I am able to query for jira issues via JQL
without my username and password but instead using client options
configured like this:

oauth_options = {
  site: 'https://my.jir.com',
  context_path: '',
  auth_type: :oauth_2legged,
  private_key_file: File.absolute_path(File.join(__dir__, 'privatekey.pem')),
  consumer_key: 'oauthtest',
  append_explicit_oauth_token_parameter: false
}

I welcome feedback on:

  • the feature flag approach taken here;
  • naming of the flag, if flagging is the correct approach to take;
  • anything else I can do to maintain consistency in the codebase or otherwise improve this request.

previously an empty oauth_token parameter was appended to the
query before sending it to jira when the auth_type parameter
was set to oauth_2legged, but it appears that oauth v0.5.5
is appending the actual value of oauth_token as a parameter

as a result the code which was appending an empty oauth_token
parameter was actually appending it to the existing oauth_token
value, thereby corrupting the token and resulting in a vague

    parameter_rejected (OAuth::Problem)

error. I had to look at the detail in the server
atlassian-jira.log file in order to see the actual problem.

in order to preserve behaviour and backwards compatibilty with
the previous release of the jira-ruby gem, this commit introduces
a new feature flag :append_explicit_oauth_token_parameter which
defaults to true, same as before, but can now be explicitly
toggled off to avoid corrupting the access token in this way.

using this commit I am able to query for jira issues via JQL
without my username and password but instead using client options
configured like this:

    oauth_options = {
      site: 'https://my.jir.com',
      context_path: '',
      auth_type: :oauth_2legged,
      private_key_file: File.absolute_path(File.join(__dir__, 'privatekey.pem')),
      consumer_key: 'oauthtest',
      append_explicit_oauth_token_parameter: false
    }
@SimonMiaou
Copy link
Collaborator

Hi @davidalpert , thank you for your suggestion.
I'm fine with a feature flag, this might be useful for some.
Could you add a test for this change?

@davidalpert
Copy link
Contributor Author

thank you @SimonMiaou; it's been a while since I looked at this change but thank you for the feedback.

let me see if I can rebase this on top of latest (to resolve conflicts) and add a test for the change.

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.

2 participants