Skip to content
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

Integration with Satellizer #143

Closed
wants to merge 31 commits into from
Closed

Integration with Satellizer #143

wants to merge 31 commits into from

Conversation

shiruken1
Copy link
Contributor

Hello again! So I've been trying to get Satellizer to work nicely, but I'm hitting a stump and would love your guidance.

Would you mind taking a look at my front-end code and pointing me in the right direction? I can't seem to get Satellizer to even attempt a login.

Many thanks!

P.S: My apologies for the gajillion commits. I've been trying to squash them all into one, but I can't seem to figure it out :(

shiruken1 and others added 25 commits January 15, 2016 10:51
# The first commit's message is:

added screencasts link

# The 2nd commit message will be skipped:

#	added favicon
@mechanicux
Copy link

Hello @shiruken1
I m using satellizer to handle auth process see if this code can help,

  • LoginController.js
    angular.module('app.controllers').controller('LoginController', LoginController);

    function LoginController($state,$auth, ToastService) {
        var vm = this;
        vm.user = {}; 
        // vm.user.email = 'Email';
        // vm.user.password = password';

        vm.login = function __login() {

            console.log(vm.user); 
            $auth.login({
                email: vm.user.email, 
                password: vm.user.password
            }).then(function (response) {
                console.log(response); 
                $state.go('app.landing');
                ToastService.show('Welcome back :) ');
            }).catch( function (response) {
                // console.log(response); 
                ToastService.error('Authentication Failed ! '); 

            });
        };


    }

and the login backend function looks like that

  • authController.php

public function login(LoginRequest $request)
    {
        $credentials = $request->only('email', 'password');
        $token = null; 
        try {
            // verify the credentials and create a token for the user
            if (! $token = JWTAuth::attempt($credentials)) {
                return response()->json(['error' => 'Invalid credentials'], Response::HTTP_UNAUTHORIZED);
            }
        } catch (JWTException $e) {
            // something went wrong
            return response()->json(['error' => 'Could not create token'], Response::HTTP_INTERNAL_SERVER_ERROR);
        }

        return response()->json(compact('token'));
    }

Good luck :)

@shiruken1
Copy link
Contributor Author

Thanks, @mechanicux! I see my coding abilities aren't half bad! 😄

The issue I'm running into is that the token stored in $localStorage is getting corrupted after some length. I've split them in 2 to highlight the difference.

response.data.token:
"eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOjEzLCJpc3MiOiJodHRwOlwvXC9sb2NhbGhvc3Q6ODAwMFwvYXV0aFwvbG9naW4iLCJpYXQiOjE0NTMxN

zQzNTIsImV4cCI6MTQ1MzE3Nzk1MiwibmJmIjoxNDUzMTc0MzUyLCJqdGkiOiJhZDc5YzAyYmVjNGI5NWFiODY5Y2NjMmRiMTE0ODQ3NSJ9.7Ru8hubpM8EFyVGw9BchWzHYgg6VYN0sujNiseBJiac"

$localStorage.jwt:
"eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOjEzLCJpc3MiOiJodHRwOlwvXC9sb2NhbGhvc3Q6ODAwMFwvYXV0aFwvbG9naW4iLCJpYXQiOjE0NTMxN

jM5NzAsImV4cCI6MTQ1MzE2NzU3MCwibmJmIjoxNDUzMTYzOTcwLCJqdGkiOiJmMTgzNGE1YzM2ZmRlNzVmMGM0YzBlYmQzZjMyOTI1ZiJ9.f-AT8PTGC2j9qg_ctQkCYYP5EHJGrxgwQbaPpkhAslQ"

I've submitted an issue to Satellizer. Here's to hoping someone knows what's going on 😦

@shiruken1
Copy link
Contributor Author

My apologies for bombarding the PR. I don't know how to post on the Trello board

@GrahamCampbell
Copy link
Contributor

Please compile the assets in production mode so that they are minified.

@shiruken1
Copy link
Contributor Author

Finally figured out what the heck is going on with Dingo

PR ready.

@shiruken1
Copy link
Contributor Author

@jadjoubran Is there anything I should do to get this PR approved? It's working, so long as the domain is defined (I have it as localhost)

@jadjoubran
Copy link
Owner

Hi @shiruken1
Thanks for your effort! Sorry I didn't have the time to check it yesterday

Unfortunately there are a few issues in this PR. Here are some of them

  • you seem to include bootstrap (or something similar) in the login-form.less which of course doesn't go well with material design and wouldn't make sense to other users
  • there are lots of unwanted diffs (.map files, exception handler, etc..)
  • You did a great job showing a demo for this, but I'm afraid of making a previous mistake that I've done before.. which is including too many pages/folders out of the box.. since most people would need to remove/modify them
  • I'm not totally sold out on the inclusion of satellizer especially that we have both Restangular and ngStorage... I know that they're different but I'm guessing we can replace satellizer with 1 or 2 lines only..
    Send me your email so I can add you to trello if you want
    But I'd rather prefer to keep all discussions on the github issues
    So feel free to open a new issue whenever you have any question 😄

@shiruken1
Copy link
Contributor Author

Hey Jad!

I definitely understand about including too many files to demo it, I was
just following your screencast of making a directive inside a feature :)

Do take a look at Satellizer's config file, though. There's definitely
going to be more than 1 or 2 lines to replace the integration with all the
social logins, but if someone can pull it off, all the better... I want to
keep this as lean and mean as possible 😃

With regards to all the diffs, I can clean that up if needed.

Let me know!
On Jan 20, 2016 3:13 AM, "Jad Joubran" notifications@github.com wrote:

Hi @shiruken1 https://github.com/shiruken1
Thanks for your effort! Sorry I didn't have the time to check it yesterday

Unfortunately there are a few issues in this PR. Here are some of them

  • you seem to include bootstrap (or something similar) in the
    login-form.less which of course doesn't go well with material design and
    wouldn't make sense to other users
  • there are lots of unwanted diffs (.map files, exception handler,
    etc..)
  • You did a great job showing a demo for this, but I'm afraid of
    making a previous mistake that I've done before.. which is including too
    many pages/folders out of the box.. since most people would need to
    remove/modify them
  • I'm not totally sold out on the inclusion of satellizer especially
    that we have both Restangular and ngStorage... I know that they're
    different but I'm guessing we can replace satellizer with 1 or 2 lines
    only.. Send me your email so I can add you to trello if you want But I'd
    rather prefer to keep all discussions on the github issues So feel free to
    open a new issue whenever you have any question [image: 😄]


Reply to this email directly or view it on GitHub
#143 (comment)
.

@shiruken1
Copy link
Contributor Author

Oh yeah, now that I see your 2nd screencast (awesome stuff!) do you want me to make a separate PR for b18e8d3?

It's about adding the domain parameter to the protected routes group, due to Dingo's issue #570

@jadjoubran
Copy link
Owner

Hi @shiruken1
Thanks for your effort!
If you could give me till the weekend as I'm swamped with work..
I'll try to open a PR without satellizer and see if we can pull it off without having to re-invent the wheel (I remember I've done something similar before)
The reason why is because this starter repo's main differentiation compared to other packages, is that it doesn't ship with too many unused dependencies

Cheers 😄

@jadjoubran
Copy link
Owner

Hi @shiruken1
I came up with a great approach to solve this ;)

After upgrading to Laravel 5.2, an artisan ng:auth would make a lot of sense.
We could generate an equivalent of what artisan make:auth would generate.
That way I was able to pull it off without Satellizer (if it was only a matter of 2 lines), it would be great

Let me know what you think

@shiruken1
Copy link
Contributor Author

I'll definitely take a look at it. Hopefully there's not much to upgrading to 5.2, in which case I'll jump into that right away! 😄

@shiruken1
Copy link
Contributor Author

@jadjoubran Definitely a great idea! Just finished watching Jeffrey Way's video on make:auth and I love it!

I won't be upgrading to 5.2 on my main project any time soon, since we're in overdrive at the moment, but would it be useful if I give it a shot in 5.1? I'm really stoked about the recipes idea! 😃

@jadjoubran
Copy link
Owner

Hi @shiruken1
Sorry I didn't get what you mean with recipes here?
The ng:auth #151 is different than the recipes

@shiruken1
Copy link
Contributor Author

Oh, I guess I misunderstood what recipes actually do. But yeah, #151 sounds
good to me!
On Jan 31, 2016 4:42 AM, "Jad Joubran" notifications@github.com wrote:

Hi @shiruken1 https://github.com/shiruken1
Sorry I didn't get what you mean with recipes here?
The ng:auth #151
#151
is different than the recipes


Reply to this email directly or view it on GitHub
#143 (comment)
.

@jadjoubran
Copy link
Owner

Awesome 😄
So I'm going to close this in favor of #151 (which is requires #121 )

Thanks for your contribution @shiruken1 🚀

@jadjoubran jadjoubran closed this Jan 31, 2016
@ghost ghost mentioned this pull request Feb 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants