Thoughts and Notes

[Michael Dugan 1240047]

I've just finished building the SSO module into our new website. It works like a charm. Overall keyboard time from start to finish was about 2.75 hours. Depending on the complexity of your web services and your skill level, it could take way less time to a bit more time.

 

What I Did:

• Our new website is being built on Laravel. With regards to SSO, I needed to do a bit of work to make it play nicely with the existing architecture.
  
• First step...config file. I extracted all the necessary config data to a file that contains other general data used by the website.
  
• I extracted sensitive information (secret and cert) to an environment file that isn't accessible over the web. I strongly recommend this - it provides a level of security that is hard to byp[Mod - Happy Thoughts] unless someone gains direct access to the filesystem.
  
• Refactoring of the SSO cl[Mod - Happy Thoughts]. I removed all constructor params, and instead pulled them directly from the config file.
  
• Separated the cl[Mod - Happy Thoughts]es from the Oauth.php into individual files. This plays more nicely with 5.3+ namespacing and cl[Mod - Happy Thoughts] autoloaders.
  
• Added `use` statements for all Oauth cl[Mod - Happy Thoughts]es in the Sso cl[Mod - Happy Thoughts]. There are 8 or 9 total cl[Mod - Happy Thoughts]es, but SSO only calls 3 of them.
  
• I added an AuthToken ORM model to byp[Mod - Happy Thoughts] a bug with session keys not being properly retrieved after the redirect. The table is truncated every 2 minutes for security
  
• Extracted the procedural session key and token setup code to static functions on the AuthToken model. One handles the initial GET request, the other handles the GET with parameters after visiting VATSIM. These functions work on "no news is good news" - if they return non-null, an error is generated.
  
• The controller instantiates the SSO cl[Mod - Happy Thoughts], and calls login/redirect functions based on the above returns.

 

Tricky Parts

• Sessions may be a minor PITA, depending on your underlying architecture
  
• isset($_GET['return']) will return false if there's no value in the URL

 

There really wasn't much "tricky" stuff to it. Including redirects and logging in/updating the user, it was +/- 100 lines of code. Most of my time was spent debugging the session keys.

 

Great job Kieran and company, can't wait to deploy this!

[Roy de Vos Burchart 116989]

Hey Michael,

 

I pretty much did exactly the same as you. Except I'm not sure if you used Laravel's built-in authentication functions, like I did. When a user logs in for the first time I just create a user in the database, otherwise I log them in using that user, using the Auth::login([model]) function, on local I use Auth::loginUsingId([my own vatsim ID]) since the IP isn't allowed.

 

Some of the files and folders:

• AuthController
  
• Config file
  
• OAuth Library

 

I'm also eagerly waiting to deploy it for public use!

 

Cheers,

Roy

[Michael Dugan 1240047]

Howdy Roy,

 

User creation is/will be handled separately since we obviously don't want just anyone with a vatsim id signing in. That particular part coordinates with the VATUSA server. I did utilize Laravel's auth package, the actual login code is:

 

//$user = $user returned from VATSIM
$dbuser = \User::find($user->user->id);
if($dbuser->is_active) \Auth::login($dbuser);
else //more code

 

I see a setter for last_login in your controller, I elected to use the updated_at timestamp to that end since each user will be automatically updated on login.

 

Our website's source code will be pubicly available/MIT licensed closer to launch, but right now I'm keeping it closed to general public since I have our staff members posting issues there. I'll try to put up the SSO related source this week when I have time though and we can compare notes

[Kieran Hardern]

[*]Refactoring of the SSO cl[Mod - Happy Thoughts]. I removed all constructor params, and instead pulled them directly from the config file.

[*]Separated the cl[Mod - Happy Thoughts]es from the Oauth.php into individual files. This plays more nicely with 5.3+ namespacing and cl[Mod - Happy Thoughts] autoloaders.

OAuth Library

 

So you've both extracted the OAuth cl[Mod - Happy Thoughts]es out of the OAuth file. Obviously the reason for including it in one is to simplify things for people that want to just include it and go, however a few I'm hoping for a couple of opinions:

 

Do you think it's worth me providing a separated copy of these (in addition to the combined)? Considering naming conventions in different frameworks, I imagine people might go through re-naming them all anyway. Is it worth it, or shall I leave as-is and let people modify to their code?

 

Looking at the cl[Mod - Happy Thoughts] names, some are perhaps a little too generic and I wouldn't like them to conflict with other cl[Mod - Happy Thoughts] names in large systems. Do you think it's worth me re-naming them to be a bit less generic, or do you think this isn't an issue?

 

 

Overall keyboard time from start to finish was about 2.75 hours

...

There really wasn't much "tricky" stuff to it. Including redirects and logging in/updating the user, it was +/- 100 lines of code. Most of my time was spent debugging the session keys.

 

Did you consider that to be a reasonable amount of time?

 

Can I make this any easier? I'm aiming for this to be workable for people with less experience than we have in this testing group, so any bits I've over-complicated would be good to tackle sooner rather than later.

[Roy de Vos Burchart 116989]

User creation is/will be handled separately since we obviously don't want just anyone with a vatsim id signing in. That particular part coordinates with the VATUSA server. I did utilize Laravel's auth package, the actual login code is:

 

...

 

I see a setter for last_login in your controller, I elected to use the updated_at timestamp to that end since each user will be automatically updated on login.

Since vataware is open to any VATSIM user, we don't need to deal with it on a registration basis. Our users are essentially all the pilots/controllers that are in our system so they share the same table and the updated_at column is also touched when the script updates either the pilot's/controller's statistics or they change some local account settings. Using the last_login gives me the option to see how many row are not set to the default value anymore and thus see how many people have ever logged in.

 

Do you think it's worth me providing a separated copy of these (in addition to the combined)? Considering naming conventions in different frameworks, I imagine people might go through re-naming them all anyway. Is it worth it, or shall I leave as-is and let people modify to their code?

You could use PSR-0 and add them to Composer/Packagist. There are enough packages (example) out there where people just have to load the composer autoload file if they don't use an awesome framework like Laravel. In the end, Composer is used more and more (for good reasons) so there shouldn't be a problem. Just give them proper namespaces like 'Vatsim\SSO\[cl[Mod - Happy Thoughts]es]' and 'Vatsim\SSO\OAuth\[OAuth cl[Mod - Happy Thoughts]es]'. It would be even cleaner to just add a base OAuth package as a dependency and extend some cl[Mod - Happy Thoughts]es them from your own package.

[Michael Dugan 1240047]

Roy beat me to it. Namespace them and create a composer package. If you use composer, it becomes easy enough to add deps in your composer.json to pull in an oauth client library instead of baking your own into it.

 

I think the time I had invested was reasonable, and was actually less considering a good part of that was debugging errors external to SSO. The setup is pretty straightforward, I don't think there's any further simplification that can be done on your end, it just becomes a matter of the developer learning how to consume the data on their end and ensure proper security.

[Michael Dugan 1240047]

I did note the rating object, which contains an id, short, long, and GRP. Is there a ref doc available that defines the possible return values? I think based on shorts and GRPs the id can be deduced, but not sure about the long titles.

[David Zhong]

The ATC rating ids are docomeented in the docomeentation for the PCSB Protocol (i.e. for the game server/clients). These are also used in the Whazzup/ServInfo files. I'm not aware of any publicly-available official docomeents.

 

These are the possible values:

1 = Obs

2 = Student 1

3 = Student 2

4 = Student 3

5 = Controller 1

6 = Controller 2

7 = Controller 3

8 = Instructor 1

9 = Instructor 2

10 = Instructor 3

11 = Supervisor

12 = Administrator

 

The names in the list are from the protocol docomeentation, but are not necessarily the official name of the rating. I think the GRP would be the authoritative source for those.

[Kieran Hardern]

As above, plus:

 

-1 = Inactive

0 = Suspended

 

If you allow those types to login of course (parameters oauth_allow_suspended, oauth_allow_inactive) in the token requests.

 

The other values are simply me outputting the 3 common conventions for naming. Personally I consider the "long" values to be the default and would encourage you to use them.

 

-1=>array('short'=>'INA', 'medium'=>'INA', 'long'=>'Inactive', 'GRP'=>'Inactive'),

0=>array('short'=>'SUS', 'medium'=>'SUS', 'long'=>'Suspended', 'GRP'=>'Suspended'),

1=>array('short'=>'OBS', 'medium'=>'OBS', 'long'=>'Pilot/Observer', 'GRP'=>'Pilot/Observer'),

2=>array('short'=>'S1', 'medium'=>'STU', 'long'=>'Student', 'GRP'=>'Tower Trainee'),

3=>array('short'=>'S2', 'medium'=>'STU2', 'long'=>'Student 2', 'GRP'=>'Tower Controller'),

4=>array('short'=>'S3', 'medium'=>'STU+', 'long'=>'Student 3', 'GRP'=>'TMA Controller'),

5=>array('short'=>'C1', 'medium'=>'CTR', 'long'=>'Controller', 'GRP'=>'Enroute Controller'),

6=>array('short'=>'C2', 'medium'=>'CTR2', 'long'=>'Controller 2', 'GRP'=>'Enroute Controller 2'),

7=>array('short'=>'C3', 'medium'=>'CTR+', 'long'=>'Senior Controller', 'GRP'=>'Senior Controller'),

8=>array('short'=>'I1', 'medium'=>'INS', 'long'=>'Instructor', 'GRP'=>'Instructor'),

9=>array('short'=>'I2', 'medium'=>'INS2', 'long'=>'Instructor 2', 'GRP'=>'Instructor 2'),

10=>array('short'=>'I3', 'medium'=>'INS+', 'long'=>'Senior Instructor', 'GRP'=>'Senior Instructor'),

11=>array('short'=>'SUP', 'medium'=>'SUP', 'long'=>'Supervisor', 'GRP'=>'Supervisor'),

12=>array('short'=>'ADM', 'medium'=>'ADM', 'long'=>'Administrator', 'GRP'=>'Administrator')

 

Edit: The GRP ones change every now and again (e.g. 'Tower Trainee' used to be 'Ground Controller', so I typically use the long versions in everything I write). I put the GRP ones in there to avoid confusion. Note that GRP only defines S1-C1, so everything around that is the long version or extrapolated (in the case of C2, though you won't need that one).

[Kieran Hardern]

Forgot to say, regarding namespace and Composer, I've added it to my to-do. Thanks for the suggestion.

 

Once again I'm hindered by the 8:30-18:00 day Mon-Fri, but I'll get it done at some point! I'm looking forward to my summer holiday in 2 weeks :p

[Michael Dugan 1240047]

Forgot to say, regarding namespace and Composer, I've added it to my to-do. Thanks for the suggestion.

 

Once again I'm hindered by the 8:30-18:00 day Mon-Fri, but I'll get it done at some point! I'm looking forward to my summer holiday in 2 weeks :p

 

If you need help, you know where to find me. I'd be glad to help with restructuring or writing some phpunit tests or whatever is needed!

[Kieran Hardern]

Can someone give these a try please to make sure they're working externally:

 

http://forums.vatsim.net/viewtopic.php?f=134&t=65319

[David Zhong]

Works from my test platform using a non-public return URL.

[Kieran Hardern]

Perfect, thanks.

[Kieran Hardern]

Forgot to say, regarding namespace and Composer, I've added it to my to-do. Thanks for the suggestion.

 

Once again I'm hindered by the 8:30-18:00 day Mon-Fri, but I'll get it done at some point! I'm looking forward to my summer holiday in 2 weeks :p

 

Forgot to mention, this will be available shortly!

[Michael Dugan 1240047]

I've just created a very boiled-down composer package, although I plan on reviewing it with Kieran and merging/updating before releasing into the wild. There's no library-specific support yet, but I 100% plan on adding a service provider and facade for Laravel in the short term.

[Roy de Vos Burchart 116989]

I've just created a very boiled-down composer package, although I plan on reviewing it with Kieran and merging/updating before releasing into the wild. There's no library-specific support yet, but I 100% plan on adding a service provider and facade for Laravel in the short term.

 

Michael,

 

We have already created a composer package (https://github.com/KHardern/VatsimSSO) but I'm just waiting for the maintainer of a dependency package to accept my pull request for a fix; if he doesn't accept it today I will fork it and create a package for that. As soon as that is done, we plan on releasing it.

 

Ours is already compatible with Laravel. Would be interesting to see if you did anything differently and then we can take the best of two packages.

 

Roy