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

Make Subject open so that it can be subclassed in Swift3 #15

Open
gabriellanata opened this issue Sep 22, 2016 · 11 comments
Open

Make Subject open so that it can be subclassed in Swift3 #15

gabriellanata opened this issue Sep 22, 2016 · 11 comments

Comments

@gabriellanata
Copy link

In Swift3 inheriting from "Subject" displays the following error:

Cannot inherit from non-open class 'Subject' outside of its defining module

To fix it the Subject class must be set to open instead of public

@edjiang
Copy link
Contributor

edjiang commented Sep 22, 2016

Hey @gabriellanata, thanks for the PR! I'd love to learn more about why you're trying to extend Subject. Subject isn't injectable in Turnstile, and I think right now it's guaranteed that the Subject API will allow you to be relatively secure =]

If there's a good reason we can open it up. Otherwise, it might be a good idea to put a "facade" on top of the Subject API for whatever integration you're making.

Happy to talk more! You can join our slack channel here: https://talkstormpath.shipit.xyz

@gabriellanata
Copy link
Author

Oh I see, I had assumed that we were supposed to have our User object subclass Subject to gain the methods automatically. Now I understand that we are supposed to have our User object be facade that interacts with Subject behind it. Sorry for the confusion, I think this is not very clear in the documentation, maybe it should be updated to reflect that. Thanks!

@edjiang
Copy link
Contributor

edjiang commented Sep 22, 2016

No problem. I'll keep this issue open until I update the docs. Unfortunately they're not the most comprehensive for integrators, but at this time the number is low =]

Just curious, what are you integrating Turnstile with?

@edjiang
Copy link
Contributor

edjiang commented Sep 22, 2016

As an example, I would recommend checking out the Perfect integration as an example of a lightweight integration. It's only ~200 LOC.

For the Perfect integration, it's using the Subject API without a facade, and I think works pretty well.

https://github.com/stormpath/Turnstile-Perfect

If you want to see how Vapor integrated it, check out: https://github.com/vapor/vapor/tree/master/Sources/Auth/Authentication

It's a little of a bigger integration, but you can see how they made request.auth a facade for the Subject, as well as wrapping the Realm and SessionManager. They did a great job there as well =]

@gabriellanata
Copy link
Author

Thanks!, it all makes more sense now.

Honestly at the moment I have not been able to decide which Swift web framework to work with, all three of the main ones (Perfect, Vapor, Kitura) have strong points. Currently i'm working with vapor just because it has the simplest api but I might decide to change later.

For that reason I'm trying to integrate most things myself and just use the web framework as the face of the server. I know this might prevent me from taking full advantage of all the power in each of the frameworks, but it reduces the amount of refactoring I have to do if I decide to switch later when the platform matures and a leader emerges.

@edjiang
Copy link
Contributor

edjiang commented Sep 22, 2016

Cool -- makes sense.

In that case, Vapor does allow you to pass in your own Turnstile instance, or pass in your own Realm.

https://github.com/vapor/vapor/blob/master/Sources/Auth/Authentication/AuthMiddleware.swift#L33

If you want to interact directly with the Subject without the Vapor facade, it's stored in request.storage["subject"], so you could add a extension like this:

extension Request {
    var subject: Subject {
        return request.storage["subject"] as! Subject
    }
}

@edjiang
Copy link
Contributor

edjiang commented Sep 22, 2016

Just to let you know, I've personally met members of all teams and they're all dedicated to making server-side Swift a thing, and have decent funding =] So I wouldn't expect Vapor, Kitura, or Perfect to go away in the near-mid future -- but of course the communities around them may change.

@nawar
Copy link

nawar commented Jan 20, 2017

Just jumping into this conversations, I've also hit this when trying to work with the Perfect's example which integrates with Turnstile. My use case is, I want to return the uniqueID when registering the user in order to use that as a foreign key in another table.

@edjiang
Copy link
Contributor

edjiang commented Jan 20, 2017

Hey @nawar, you should be able to access the uniqueID by calling subject.authDetails?.account?.uniqueID. What would you want to do by subclassing Subject?

@nawar
Copy link

nawar commented Jan 20, 2017

@edjiang I tried that already. In Perfect's Realm implementation, they are trying to return the Account in their register() and authenticate() functions as you can see below

public func authenticate(credentials: AccessToken) throws -> Account {
        let account = AuthAccount()
        let token = AccessTokenStore()
        do {
                try token.get(credentials.string)
                if token.check() == false {
                        throw IncorrectCredentialsError()
                }
                try account.get(token.userid)
                return account
        } catch {
                throw IncorrectCredentialsError()
        }
}
// ...
public func register(credentials: Credentials) throws -> Account {

        let account = AuthAccount()
        let newAccount = AuthAccount()
        newAccount.id(String(random.secureToken))

        switch credentials {
        case let credentials as UsernamePassword:
                do {
                        if account.exists(credentials.username) {
                                throw AccountTakenError()
                        }
                        newAccount.username = credentials.username
                        newAccount.password = credentials.password
                        do {
                                try newAccount.make() // can't use save as the id is populated
                        } catch {
                                print("REGISTER ERROR: \(error)")
                        }
                } catch {
                        throw AccountTakenError()
                }
        default:
                throw UnsupportedCredentialsError()
        }
        return newAccount
}

And in your implementation of Subject, you are maintaining Account in the login() function by adding it to authDetails and discarding it in register().

 public func login(credentials: Credentials, persist: Bool = false) throws {
        let account = try turnstile.realm.authenticate(credentials: credentials)
        let sessionID: String? = persist ? turnstile.sessionManager.createSession(account: account) : nil
        let credentialType = type(of: credentials)
        
        authDetails = AuthenticationDetails(account: account, sessionID: sessionID, credentialType: credentialType)
    }
    
    /// Another one bites the dust.. 
    public func register(credentials: Credentials) throws {
        _ = try turnstile.realm.register(credentials: credentials)
    }

Following with your suggestion, this only worked with the login() function. I even cast Auth to AuthAccount and get access to the other properties of AuthAccount

if let authAccount = request.user.authDetails?.account as? AuthAccount {
// ...
}

So I think, the better suggestion is to let register() assigns the authDetails instance as well.

@edjiang
Copy link
Contributor

edjiang commented Jan 20, 2017

Gotcha. If you want the user to be logged in after registration, I would call register and login in quick succession. That would set the "authDetails".

For instance: https://github.com/stormpath/Turnstile-Perfect-Example/blob/master/Sources/main.swift#L74

The Subject API is meant to be a guide to the current authentication state of your application. Not all applications will have registration + login in one step, especially if there's a verification process. Thus, setting the authDetails is not a good idea.

Just in general, from the docs:

The Subject API in Turnstile also supports registration, however this is a convenience for basic use cases. Since different apps have different rules on registration and user managment, it's expected that you will most likely write your own registration and user management logic.

I'm unfamiliar with how the Perfect implementation you're using works, but I'm guessing you can call register directly on the Realm if you want to get the full account information.

Hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants