{"action":"created","issue":{"url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/155","repository_url":"https://api.github.com/repos/aacotroneo/laravel-saml2","labels_url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/155/labels{/name}","comments_url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/155/comments","events_url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/155/events","html_url":"https://github.com/aacotroneo/laravel-saml2/pull/155","id":356433597,"node_id":"MDExOlB1bGxSZXF1ZXN0MjEyNjk1Njk2","number":155,"title":"Changes to project structure and code cleanup","user":{"login":"tjallingt","id":5736783,"node_id":"MDQ6VXNlcjU3MzY3ODM=","avatar_url":"https://avatars1.githubusercontent.com/u/5736783?v=4","gravatar_id":"","url":"https://api.github.com/users/tjallingt","html_url":"https://github.com/tjallingt","followers_url":"https://api.github.com/users/tjallingt/followers","following_url":"https://api.github.com/users/tjallingt/following{/other_user}","gists_url":"https://api.github.com/users/tjallingt/gists{/gist_id}","starred_url":"https://api.github.com/users/tjallingt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tjallingt/subscriptions","organizations_url":"https://api.github.com/users/tjallingt/orgs","repos_url":"https://api.github.com/users/tjallingt/repos","events_url":"https://api.github.com/users/tjallingt/events{/privacy}","received_events_url":"https://api.github.com/users/tjallingt/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-09-03T09:50:12Z","updated_at":"2018-09-03T14:39:47Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/aacotroneo/laravel-saml2/pulls/155","html_url":"https://github.com/aacotroneo/laravel-saml2/pull/155","diff_url":"https://github.com/aacotroneo/laravel-saml2/pull/155.diff","patch_url":"https://github.com/aacotroneo/laravel-saml2/pull/155.patch"},"body":"This sort of depends on/overwrites https://github.com/aacotroneo/laravel-saml2/pull/151\r\n\r\nI modernised the project structure to follow psr-4 (instead of psr-0) and removed a bunch of IMO strange code. Also updated the tests, refactored the service provider and added more tests. Some of my changes might have been too aggressive (composer.json php version and changes to config for example) but we can discuss what will and won't work based around the contents of this PR.\r\n\r\nThere are a couple of other changes I'm considering:\r\n\r\n- I'd like to remove the OneLogin singleton registration from the service provider as it isn't adding anything. This library's Auth class should probably receive the Laravel config, massage it into the right format and create a OneLogin instance.\r\n- OneLogin seems to already strip the leading and ending tags from openssl certificates and private keys so maybe the ExtractOpenssl class can be removed entirely at some point. (see https://github.com/onelogin/php-saml/blob/3.0.0/src/Saml2/Utils.php#L189-L272)\r\n- be stricter about how the certificates and private key are configured; I dislike pasting the entire contents in the `.env` file and the use of `file://` is also not my favourite so maybe we can just specify a folder and only configure filenames?\r\n- Use Laravel Redirects instead of relying on OneLogin to redirect us.\r\n\r\nEven larger changes I'd like to see eventually:\r\n\r\n- Don't use an event to signal a login attempt\r\n- Instead of having users configure this manually supply a trait like Laravel's default `AuthorizesUsers` on the login controller\r\n\r\nThoughts and input are very welcome 😄 \r\n"},"comment":{"url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/comments/418134353","html_url":"https://github.com/aacotroneo/laravel-saml2/pull/155#issuecomment-418134353","issue_url":"https://api.github.com/repos/aacotroneo/laravel-saml2/issues/155","id":418134353,"node_id":"MDEyOklzc3VlQ29tbWVudDQxODEzNDM1Mw==","user":{"login":"aacotroneo","id":10578291,"node_id":"MDQ6VXNlcjEwNTc4Mjkx","avatar_url":"https://avatars2.githubusercontent.com/u/10578291?v=4","gravatar_id":"","url":"https://api.github.com/users/aacotroneo","html_url":"https://github.com/aacotroneo","followers_url":"https://api.github.com/users/aacotroneo/followers","following_url":"https://api.github.com/users/aacotroneo/following{/other_user}","gists_url":"https://api.github.com/users/aacotroneo/gists{/gist_id}","starred_url":"https://api.github.com/users/aacotroneo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aacotroneo/subscriptions","organizations_url":"https://api.github.com/users/aacotroneo/orgs","repos_url":"https://api.github.com/users/aacotroneo/repos","events_url":"https://api.github.com/users/aacotroneo/events{/privacy}","received_events_url":"https://api.github.com/users/aacotroneo/received_events","type":"User","site_admin":false},"created_at":"2018-09-03T14:39:47Z","updated_at":"2018-09-03T14:39:47Z","author_association":"OWNER","body":"I get your point.. I see the value on it, but we can't drop support for our current users, and both approaches have some deep differences in design. We'll need to keep updating this library a little bit to add support to newer PHP versions and keep up to date with oneLogin - I don't think we can make both compatible.\r\n\r\nMay be you can do a fork `tjallingt/laravel-simple-saml2?` (or the name you like) and we can update the REDME to explain how both libraries relate. I'd be happy to help users migrate towards your library to take more advantage of Laravel. I'm not using Laravel right now, so I can't give more support than maintaining what we have."}}
{"actor":{"display_login":"aacotroneo"}}