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

Enable addition OAuth 2.0 grant flows #83

Merged
merged 1 commit into from
Nov 14, 2014
Merged

Enable addition OAuth 2.0 grant flows #83

merged 1 commit into from
Nov 14, 2014

Conversation

blakeembrey
Copy link
Contributor

No description provided.

@blakeembrey blakeembrey force-pushed the oauth2-grants branch 3 times, most recently from 63b4d40 to 4a56bd5 Compare November 14, 2014 00:58
var authorization = btoa(credentials.clientId + ':' + credentials.clientSecret);

return {
'Authorization': 'Bearer' + authorization
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing space after Bearer?

@xaka
Copy link
Contributor

xaka commented Nov 14, 2014

Any thoughts about failed build? Just a noise?

@dmartinezg
Copy link

The broken build is because of this:
angular/angular.js#7874

In angular 1.2.16, they fixed some bugs, which the api-console relies on for rendering the (+) button at the last item of a repeatable named parameter: https://github.com/mulesoft/api-console/blob/master/app/views/repeatable.tmpl.html#L6

After that patch, the transcluded items do not share the same scope as the transcluder, and the the $last scope variable is no longer present in the transcluded scope.

To fix this we would need a semi-major overhaul o named parameters.


<div class="control-group" ng-if="credentials.grantType.type !== 'token'">
<label for="clientSecret">
<span class="required" ng-show="credentials.grantType.type !== 'owner'">*</span>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be cleaner to do something like:

In CSS:

.required:after {
    content: "*";
}

and then

<label for="clientSecret" ng-style:"{required: requiresSecret}">

and add to the controller:

$scope.requiresSecret = credentials.grantType.type !== 'token';

and remove the span?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, I was just using what had already been implemented though.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let me rephrase:

I think we should do something like:

In CSS:

.required:after {
    content: "*";
}

and then

<label for="clientSecret" ng-style:"{required: requiresSecret}">

and add to the controller:

$scope.requiresSecret = credentials.grantType.type !== 'token';

and remove the span.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that change is now a part of this PR. It only changes one part of unrelated code.

@blakeembrey blakeembrey force-pushed the oauth2-grants branch 3 times, most recently from 05e7f37 to 4b59917 Compare November 14, 2014 17:27
@dmartinezg
Copy link

LGTM

@dmartinezg
Copy link

@blakeembrey When you merge this one, can you also update the console version in api-designer and send me the SHAs, so I can update the tooling in our side?

@xaka
Copy link
Contributor

xaka commented Nov 14, 2014

Don't forget to re-build console and designer.

blakeembrey added a commit that referenced this pull request Nov 14, 2014
Enable addition OAuth 2.0 grant flows
@blakeembrey blakeembrey merged commit 806aa78 into master Nov 14, 2014
@blakeembrey blakeembrey deleted the oauth2-grants branch November 14, 2014 19:46
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.

3 participants