This repository has been archived by the owner. It is now read-only.

JOAuth 2.0 Client #1423

Closed
wants to merge 23 commits into
from

Conversation

Projects
None yet
6 participants
@aaronschmitz
Contributor

aaronschmitz commented Aug 2, 2012

This is an OAuth 2.0 client for requesting access tokens and sending signed requests. I created it as a part of my JGoogle GSoC project (http://goo.gl/5dj50).

@aaronschmitz

This comment has been minimized.

Show comment
Hide comment
@aaronschmitz

aaronschmitz Aug 2, 2012

Contributor

See also Diana's 1.0a client: #1422

Contributor

aaronschmitz commented Aug 2, 2012

See also Diana's 1.0a client: #1422

libraries/joomla/oauth/oauth2client.php
+
+defined('JPATH_PLATFORM') or die;
+jimport('joomla.environment.response');
+jimport('joomla.environment.uri');

This comment has been minimized.

@realityking

realityking Aug 2, 2012

Contributor

The jimport for JUri isn't necessary anymore.

@realityking

realityking Aug 2, 2012

Contributor

The jimport for JUri isn't necessary anymore.

libraries/joomla/oauth/oauth2client.php
+ * @subpackage Oauth
+ * @since 12.2
+ */
+class JOauthOauth2client

This comment has been minimized.

@stefanneculai

stefanneculai Aug 2, 2012

Contributor

I find it strange to name the class JOauthOauth2client. Wouldn't it be better to change the name to JOauth2client (it sounds more natural)?

@stefanneculai

stefanneculai Aug 2, 2012

Contributor

I find it strange to name the class JOauthOauth2client. Wouldn't it be better to change the name to JOauth2client (it sounds more natural)?

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Aug 2, 2012

Contributor

Then wouldn't the autoloader be looking for a file named 2client?

Contributor

elinw commented Aug 2, 2012

Then wouldn't the autoloader be looking for a file named 2client?

@stefanneculai

This comment has been minimized.

Show comment
Hide comment
@stefanneculai

stefanneculai Aug 3, 2012

Contributor

Would be a problem to name it 2client.php?
Maybe make a folder called v2 and put the client.php in it. This way it would be JOAuthV2client.

Contributor

stefanneculai commented Aug 3, 2012

Would be a problem to name it 2client.php?
Maybe make a folder called v2 and put the client.php in it. This way it would be JOAuthV2client.

@aaronschmitz

This comment has been minimized.

Show comment
Hide comment
@aaronschmitz

aaronschmitz Aug 4, 2012

Contributor

I suppose it wouldn't be a problem.

Contributor

aaronschmitz commented Aug 4, 2012

I suppose it wouldn't be a problem.

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Aug 4, 2012

Contributor

I think I'd like to hear from the maintainers on this one.

Just FYI Aaron needs to know about the patch() issue in order to know how to fix the unit test that has an error.

Contributor

elinw commented Aug 4, 2012

I think I'd like to hear from the maintainers on this one.

Just FYI Aaron needs to know about the patch() issue in order to know how to fix the unit test that has an error.

@aaronschmitz

This comment has been minimized.

Show comment
Hide comment
@aaronschmitz

aaronschmitz Aug 5, 2012

Contributor

Ok. I switch to extending JHttp like JGithub does. If the maintainers accept @dianaprajescu's pull for the patch method I'm 95% of the way there, and if they don't this is a superior alternative to using JHttpTransport.

For now, I renamed the class to JOauth2client and the file to 2client, but I'm flexible on what others prefer.

Contributor

aaronschmitz commented Aug 5, 2012

Ok. I switch to extending JHttp like JGithub does. If the maintainers accept @dianaprajescu's pull for the patch method I'm 95% of the way there, and if they don't this is a superior alternative to using JHttpTransport.

For now, I renamed the class to JOauth2client and the file to 2client, but I'm flexible on what others prefer.

@aaronschmitz

This comment has been minimized.

Show comment
Hide comment
@aaronschmitz

aaronschmitz Sep 11, 2012

Owner

@LouisLandry, is this what you were thinking?

Owner

aaronschmitz commented on f468100 Sep 11, 2012

@LouisLandry, is this what you were thinking?

This comment has been minimized.

Show comment
Hide comment
@LouisLandry

LouisLandry Sep 12, 2012

That's fine, but you'd also have to call $application->sendHeaders(); I believe. Is there some reason that you couldn't just use $application->redirect() and needed to do it this way?

That's fine, but you'd also have to call $application->sendHeaders(); I believe. Is there some reason that you couldn't just use $application->redirect() and needed to do it this way?

@pasamio

This comment has been minimized.

Show comment
Hide comment
@pasamio

pasamio Sep 24, 2012

Contributor

@aaronschmitz do you mind renaming it from oauth/v2client.php to oauth2/client.php? Given it seems to be an independent implementation with no sharing of code between the two implementations.

Contributor

pasamio commented Sep 24, 2012

@aaronschmitz do you mind renaming it from oauth/v2client.php to oauth2/client.php? Given it seems to be an independent implementation with no sharing of code between the two implementations.

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Sep 28, 2012

Contributor

@pasamio can you comment on whether the class name should be JOAuth or JOauth (my understanding is that the loader looks for non consecutive upper case letters which would make JOAuth okay. Codestyle originally called for uppercase where appropriate (HTML ZXML) but we veered away from that with implementation of the autoloader. We probably should aim for some consistency.

Contributor

elinw commented Sep 28, 2012

@pasamio can you comment on whether the class name should be JOAuth or JOauth (my understanding is that the loader looks for non consecutive upper case letters which would make JOAuth okay. Codestyle originally called for uppercase where appropriate (HTML ZXML) but we veered away from that with implementation of the autoloader. We probably should aim for some consistency.

@aaronschmitz

This comment has been minimized.

Show comment
Hide comment
@aaronschmitz

aaronschmitz Oct 4, 2012

Contributor

Closing in favor of #1480.

Contributor

aaronschmitz commented Oct 4, 2012

Closing in favor of #1480.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.