FBXMPP OAuth

[Frank D]

On 2011-03-01, at 1:24 AM, Peter Hosey wrote:

> On Feb 28, 2011, at 17:55:36, Colin Barrett^[c2bFrank Dowsett wrote:
>> <xmppFB.diff>
> 
> Return of the JSON framework!
Different one, smaller in size, I believe.

>> +- (void)doIT
> 
> Surely we can think of a better method name than that.
As I wrote to Colin this "is my unfinished, nowhere-near-release-ready coding."

> 
>> +		token = @"176717249009197|2.SBY9kCilxdgHv6dRMAPZvQ__.86400.1297969200-899650296|AA0KSXp7kShNmKEC5PzAHdo6O50";
> 
> What is this? What is its origin/how was it made?
> 
> This sort of thing should be documented in a variable name and/or comment.
That was my FB token so that I didn't have to login.

> 
>> +	NSString *token = [adium.accountController passwordForAccount:account];
> 
>> +	NSString *urlstring = [NSString stringWithFormat:@"https://graph.facebook.com/me?access_token=%@", token];
> 
> So, if the user sets a password, it will be passed to Facebook in clear text as a GET parameter? Is this necessary, or can we do better?
The password is the access token.

>> +	NSDictionary *resp = [[[NSString alloc] initWithData:conn encoding:NSUTF8StringEncoding] JSONValue];
> 
> Leak.
I know.

> 
>> +        [[NSNotificationCenter defaultCenter] postNotificationName:FACEBOOK_OAUTH_FINISHED
> 
>> +		object:[self parseURLParams:[[request URL] fragment]]];
> 
>> +	NSString *token = [[note object] objectForKey:@"access_token"];
> 
> Are you sure you want fragment and not queryString?
Yes.

> I think an example of the sort of URL we're parsing here, stored in a comment in the redirect method, is warranted.
I agree.

> [snip]
See second comment.

> Should this be a subclass of ESJabberService?
I tried it at the start as a subclass, but there was a lot more going on than I thought needed to be before I could even login.

>> +- (NSCharacterSet *)allowedCharacters{
>> +	return [NSCharacterSet characterSetWithCharactersInString:@"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_ "];
>> +}
>> +
>> +- (NSCharacterSet *)allowedCharactersForUIDs{
>> +	return [NSCharacterSet characterSetWithCharactersInString:@"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_"];
>> }
> 
> Do we really only want the ASCII letters and numbers, or would it be better to use the alphanumeric character set and add the ‘_’ and space to it?
> 
> Also, is that space supposed to be there? Do we only want the space, or should we allow all whitespace? What about line breaks?
The space is for the formattedUID to show a pretty name.

> If we make this a subclass of ESJabberService, should we inherit its -allowedCharacters and -allowedCharactersForUIDs methods?
Probably depends on how we want the account name displayed.

>> +GCC_VERSION = com.apple.compilers.llvm.clang.1_0
> 
> Is this meant to be included in this patch?
Eventually, but in a different patch. I didn't go through the patch at all.

--
Frank
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20110301/ce217468/attachment-0002.html>