-
Notifications
You must be signed in to change notification settings - Fork 46
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
Allow commenting on Gistlog without leaving UI #78
Changes from 16 commits
7e69229
12fc7a8
f16c308
e239a6d
688354a
afc259e
1eda04c
9ce869c
61edb00
1d92395
d7c5b0b
564b50f
c3c14d5
3d4f38b
c7a95a1
a7688c0
31323cd
4aea31b
08afb6d
995ba32
fecce4a
d1ce148
44fd732
c7e5f53
06317e6
553539f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,92 @@ | ||
<?php namespace Gistlog\Http\Controllers\Auth; | ||
|
||
use Gistlog\Http\Controllers\Controller; | ||
use Illuminate\Contracts\Auth\Guard; | ||
use Illuminate\Contracts\Auth\Registrar; | ||
use Gistlog\User; | ||
use Exception; | ||
use Illuminate\Foundation\Auth\AuthenticatesAndRegistersUsers; | ||
use Illuminate\Foundation\Auth\ThrottlesLogins; | ||
use Illuminate\Support\Facades\Auth; | ||
use Illuminate\Support\Facades\Redirect; | ||
use Illuminate\Support\Facades\Session; | ||
use Illuminate\Support\Facades\URL; | ||
use Laravel\Socialite\Facades\Socialite; | ||
use Validator; | ||
|
||
class AuthController extends Controller { | ||
|
||
/* | ||
|-------------------------------------------------------------------------- | ||
| Registration & Login Controller | ||
|-------------------------------------------------------------------------- | ||
| | ||
| This controller handles the registration of new users, as well as the | ||
| authentication of existing users. By default, this controller uses | ||
| a simple trait to add these behaviors. Why don't you explore it? | ||
| | ||
*/ | ||
|
||
use AuthenticatesAndRegistersUsers; | ||
|
||
/** | ||
* Create a new authentication controller instance. | ||
* | ||
* @param \Illuminate\Contracts\Auth\Guard $auth | ||
* @param \Illuminate\Contracts\Auth\Registrar $registrar | ||
* @return void | ||
*/ | ||
public function __construct(Guard $auth, Registrar $registrar) | ||
{ | ||
$this->auth = $auth; | ||
$this->registrar = $registrar; | ||
|
||
$this->middleware('guest', ['except' => 'getLogout']); | ||
} | ||
class AuthController extends Controller | ||
{ | ||
use AuthenticatesAndRegistersUsers, ThrottlesLogins; | ||
|
||
/** | ||
* Create a new authentication controller instance. | ||
* | ||
* @return void | ||
*/ | ||
public function __construct() | ||
{ | ||
$this->middleware('guest', ['except' => 'getLogout']); | ||
} | ||
|
||
/** | ||
* Redirect the user to the GitHub authentication page. | ||
* | ||
* @return Response | ||
*/ | ||
public function redirectToProvider() | ||
{ | ||
session()->put('redirect_to', URL::previous()); | ||
return Socialite::driver('github') | ||
->scopes(['gist']) | ||
->redirect(); | ||
} | ||
|
||
/** | ||
* Obtain the user information from GitHub. | ||
* | ||
* @return Response | ||
*/ | ||
public function handleProviderCallback() | ||
{ | ||
try { | ||
$user = Socialite::driver('github')->user(); | ||
} catch (Exception $e) { | ||
return Redirect::to('auth/github'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we give error messages here? |
||
} | ||
|
||
$authUser = $this->findOrCreateUser($user); | ||
|
||
Auth::login($authUser, true); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra line |
||
|
||
return redirect($this->referringUrl()); | ||
} | ||
|
||
private function findOrCreateUser($user) | ||
{ | ||
if ($authUser = User::where('github_id', $user->id)->first()) { | ||
return $authUser; | ||
} | ||
|
||
return User::create([ | ||
'name' => $user->name, | ||
'email' => $user->email, | ||
'github_id' => $user->id, | ||
'avatar' => $user->avatar, | ||
'token' => $user->token | ||
]); | ||
} | ||
|
||
public function getLogout() | ||
{ | ||
Auth::logout(); | ||
|
||
return redirect('/'); | ||
} | ||
|
||
private function referringUrl() | ||
{ | ||
$redirect_to = session()->get('redirect_to', 'home'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason we don't just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to flash, but out of curiosity, would flashing the session survive the OAuth ping pong that goes on, or does that all happen in the background? I think that was my original purpose in using put and get on the session. I tested it out and it seemed to work fine though so I'll stick with flash for now I suppose. |
||
session()->forget('redirect_to'); | ||
return $redirect_to; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ class Kernel extends HttpKernel { | |
'auth' => 'Gistlog\Http\Middleware\Authenticate', | ||
'auth.basic' => 'Illuminate\Auth\Middleware\AuthenticateWithBasicAuth', | ||
'guest' => 'Gistlog\Http\Middleware\RedirectIfAuthenticated', | ||
'csrf' => 'Gistlog\Http\Middleware\VerifyCsrfToken', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation issues but doesn't really bother me all that much since we're bout to re-indent the whole thing anyway |
||
]; | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,6 +148,7 @@ | |
|
||
'Gistlog\Providers\GistClientServiceProvider', | ||
'Gistlog\Providers\ContentParserServiceProvider', | ||
Laravel\Socialite\SocialiteServiceProvider::class | ||
], | ||
|
||
/* | ||
|
@@ -195,6 +196,7 @@ | |
'URL' => 'Illuminate\Support\Facades\URL', | ||
'Validator' => 'Illuminate\Support\Facades\Validator', | ||
'View' => 'Illuminate\Support\Facades\View', | ||
'Socialite' => Laravel\Socialite\Facades\Socialite::class, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spaces must be used to indent lines; tabs are not allowed |
||
], | ||
|
||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,12 +12,15 @@ class CreateUsersTable extends Migration { | |
*/ | ||
public function up() | ||
{ | ||
Schema::create('users', function(Blueprint $table) | ||
{ | ||
Schema::create('users', function (Blueprint $table) { | ||
$table->increments('id'); | ||
|
||
$table->string('github_id')->unique(); | ||
$table->string('name'); | ||
$table->string('email')->unique(); | ||
$table->string('password', 60); | ||
$table->string('email'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried running this locally and it didn't bring back an email. Not sure why, but I think we need to make email and name nullable, basd on my experiences with giscus There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't have the |
||
$table->string('avatar'); | ||
$table->string('token'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have live data already, you'll have to add this modification as a new migration; otherwise it won't be run on the live site. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (hm, strike that.. it kinda looks like there's ... no actual database so far? HA. Nevermind. This is fine.)... we'll just need to remember to actually add a db before we merge this one |
||
|
||
$table->rememberToken(); | ||
$table->timestamps(); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alphabetize use block plz