Store and use access token with each network request
alchemistgo87 opened this issue · 10 comments
Hi @mgacy
In order to make my api work, I need to store access token which I get after calling login api. I am storing this access token in the UserStorageManager. Now, I need to use this token in every request (except Login and Signup requests)
For this to work, I would need to somehow obtain access token inside ApiClient, so that I can add it to the configuration of SessionManager. So, I have 2 questions regarding the same:
- Correct way to obtain Access Token from UserStorageManager inside ApiClient.
- Correct way to Add separate headers for Login/Signup requests and the rest of the api requests, inside ApiClient.
Thank you.
First, a disclaimer: I do not claim to be a security expert and any full solution should depend on the specifics of your implementation. Now, with that out of the way …
I would advise against using UserStorageManager to store your token. Instead, create a CredentialsManager which uses Keychain Services to store it securely. You might want to use a Keychain wrapper so you don't have to deal with Keychain directly; I like KeychainAccess.
Use an object conforming to Alamofire's RequestAdapter protocol to add the token to your request headers. Take a look at the AccessTokenAdapter and OAuth2Handler classes in the Adapting and Retrying Requests section of Alamofire's documentation for a good starting point. You could then add a method to APIClient (and ClientType or another protocol to which it conforms) allowing you to pass this object to SessionManager:
class APIClient: ClientType {
// MARK: Properties
private let sessionManager: SessionManager
// ...
func configSession(_ adapter: RequestAdapter?) {
sessionManager.adapter = adapter
}
// ...
}Alternatively, you could make an AutheticationHandler with its own SessionManager, similar to the OAuth2Handler mentioned above, which UserManager could use to handle login and signup requests.
Sent with GitHawk
If you modified UserManager.init() to accept APIClient as a parameter and initialized it with the same instance you used for your various services, they would also end up adding the token to requests after you call APIClient.configSession(with:) in UserManager. In this case, I would do something like the following to ensure only UserManager is able to modify APIClient.sessionManager:
protocol ClientType {
func request<T: Codable>(_: URLRequestConvertible) -> Single<T>
func requestImage(_ endpoint: URLRequestConvertible) -> Single<UIImage>
}
protocol ConfigurableClientType: ClientType {
func configSession(with adapter: RequestAdapter?)
}
class APIClient: ClientType {
// ...
}
extension APIClient: ConfigurableClientType {
func configSession(with adapter: RequestAdapter?) {
sessionManager.adapter = adapter
}
}
// ...
class UserManager {
var authenticationState: AuthenticationState
let currentUser = BehaviorSubject<User?>(value: nil)
private let client: ConfigurableClientType
private let storageManager: UserStorageManagerType
init(client: ConfigurableClientType) {
self.client = client
self.storageManager = UserStorageManager()
if let user = storageManager.read() {
self.authenticationState = .signedIn
self.currentUser.onNext(user)
} else {
self.authenticationState = .signedOut
}
}
}
// ...
struct AppDependency: HasClient, HasUserManager, HasAlbumService, HasPostService, HasTodoService {
let client: APIClient
let userManager: UserManager
let albumService: AlbumServiceType
let postService: PostServiceType
let todoService: TodoServiceType
init() {
self.client = APIClient()
self.userManager = UserManager(client: client)
self.albumService = AlbumService(client: client)
self.postService = PostService(client: client)
self.todoService = TodoService(client: client)
}
}You could also do something similar while only sharing SessionManager.
Sent with GitHawk
Thank you so much @mgacy for your response! This is really helpful.
The only question now I have is that I have about 20 different services (similar to AlbumService, PostService etc.) which can run after the user has logged-in. Each of these services requires to infuse SessionManager with same kind of RequestAdapter (which will basically put the Authorization Token, received after login, into the header). In short, there are just two types of RequestAdapters, one is to be used inside UserManager and the other is to be used inside all the other services. So, is there a way to save the overhead of infusing same kind of RequestAdapters in all the 20 services?
Thank you again!
Actually, I am thinking there will only be a single instance of one class of RequestAdapter. When it is .signedOut, UserManager won't use a RequestAdapter. Once it gets a token, UserManager will create a RequestAdapter with that token and pass it to the SessionManager of its APIClient using configSession(with:). Since you instantiate your UserManager and all of your services with a single APIClient (or just the SessionManager), and these are reference types, all requests should now have the token in their header.
I would expect that upon logout, you could simply pass nil to configSession(with:).
Sent with GitHawk
So are you suggesting me something like this:
class UserManager {
var authenticationState: AuthenticationState
let currentUser = BehaviorSubject<User?>(value: nil)
private let client: ConfigurableClientType
private let storageManager: UserStorageManagerType
private let accessTokenAdapter: AccessTokenAdapter
init(client: ConfigurableClientType) {
self.client = client
self.storageManager = UserStorageManager()
self.accessTokenAdapter = AccessTokenAdapter(accessToken: "ACCESS_TOKEN")
if let user = storageManager.read() {
client.configSession(with: accessTokenAdapter)
self.authenticationState = .signedIn
self.currentUser.onNext(user)
} else {
client.configSession(with: nil)
self.authenticationState = .signedOut
}
}
}
Also, in the login and logout apis, the sessionManager will be dynamically updated like this:
func logout() -> Single<Bool> {
// just a mock
return Single.just(true)
.delay(0.5, scheduler: MainScheduler.instance)
.do(onSuccess: { [weak self] _ in
self?.client.configSession(with: nil)
self?.authenticationState = .signedOut
self?.storageManager.clear()
self?.currentUser.onNext(nil)
})
}
func login(username: String, password: String) -> Single<Bool> {
return client.request(Router.getUser(id: 1))
.do(onSuccess: { [weak self] (user: User) in
self?.client.configSession(with: accessTokenAdapter)
self?.authenticationState = .signedIn
self?.currentUser.onNext(user)
self?.storageManager.store(user: user)
})
.map { _ in return true }
}
Getting there, but don't forget about the CredentialsManager we're using to store our access token in the keychain. So, we do need to maintain a reference to that, but I'm not sure we need one for our AccessTokenAdapter. We probably shouldn't modify AccessTokenAdapter after creating it and Alamofire.SessionManager (or rather, the Alamofire.SessionDelegate belonging to it) maintains a reference to it. So, I'm thinking something more like this:
class UserManager {
var authenticationState: AuthenticationState
let currentUser = BehaviorSubject<User?>(value: nil)
private let client: ConfigurableClientType
private let storageManager: UserStorageManagerType
private let credentialsManager: CredentialsManagerType
init(client: ConfigurableClientType) {
self.client = client
self.storageManager = UserStorageManager()
self.credentialsManager = CredentialsManager()
if let accessToken = credentialsManager.read() {
self.authenticationState = .signedIn
let requestAdapter = AccessTokenAdapter(accessToken: accessToken)
client.configSession(with: requestAdapter)
// This could definitely be improved
if let user = storageManager.read() {
self.currentUser.onNext(user)
} else {
client.request(Router.getUser())
.do(onSuccess: { [weak self] (user: User) in
self?.currentUser.onNext(user)
self?.storageManager.store(user: user)
})
}
} else {
self.authenticationState = .signedOut
}
}
}For login, the response to our request should contain the access token, which we will save to CredentialsManager and use to create an AccessTokenAdapter that we'll pass to APIClient.configSession(with:).
Edit: fix init
Sent with GitHawk
Got it, looks much better. But in the init method, should we initialize client with a new instance of ApiClient or should we use the one coming in as a parameter of init?
This is how I have modified my code:
class UserManager {
var authenticationState: AuthenticationState
var accessDetail: Credentials?
let currentUser = BehaviorSubject<User?>(value: nil)
private let client: ConfigurableClientType
private let storageManager: UserStorageManagerType
private let credentialsManager: CredentialsManagerType
init(client: ConfigurableClientType) {
self.client = client
self.storageManager = UserStorageManager()
self.credentialsManager = CredentialsManager()
if let credentials = credentialsManager.read() {
self.authenticationState = .signedIn
let requestAdapter = AccessTokenAdapter(accessToken: credentials.access_token!)
client.configSession(with: requestAdapter)
if let user = storageManager.read() {
self.currentUser.onNext(user)
} else {
self.getCurrentUser()
}
} else {
self.authenticationState = .signedOut
}
}
}
extension UserManager: LoginService {
func login(username: String, password: String) -> Single<Bool> {
let loginAdapter = LoginAdapter()
client.configSession(with: loginAdapter)
return client.request(LoginApi.login(password: password, username: username,
clientId: "MY_CLIENT_ID", clientSecret: "MY_CLIENT_SECRET",
scope: "read write", grantType: "password"))
.do(onSuccess: { [weak self] (credentials: Credentials) in
self?.credentialsManager.store(credentials: credentials)
let requestAdapter = AccessTokenAdapter(accessToken: credentials.access_token!)
self?.client.configSession(with: requestAdapter)
self?.authenticationState = .signedIn
self?.getCurrentUser()
})
.map { _ in return true }
}
}
extension UserManager: CurrentUserService {
func getCurrentUser() -> Single<Bool> {
return client.request(LoginApi.getCurrentUser)
.do(onSuccess: { [weak self] (user: User) in
self?.storageManager.store(user: user)
self?.currentUser.onNext(user)
})
.map { _ in return true}
}
}
In login function, I am adding a new RequestAdapter named LoginAdapter to infuse session manager with separate headers required for login api. Is login function the correct place to do so, or should I do this after the logout and while initializing the UserManager?
Also, apart from username and password, I need to pass some more query parameters like clientId, clientSecret in the login request, can these extra parameters be added using the LoginAdapter?
Currently my LoginAdapter looks like this:
class LoginAdapter: RequestAdapter {
func adapt(_ urlRequest: URLRequest) throws -> URLRequest {
var urlRequest = urlRequest
if let urlString = urlRequest.url?.absoluteString, urlString.hasPrefix(ApiConst.baseUrl) {
urlRequest.setValue("Basic xxxxxxxxxxxxxxxxx" , forHTTPHeaderField: "Authorization")
urlRequest.setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type")
}
return urlRequest
}
}
You could do things that way, but it doesn't really feel right to me. RequestAdapter strikes me as a convenient way to make the same changes to a number of different requests, rather than modify a single one. Also, modifications to the SessionManager are shared, so if login fails and any of your services happen to make a request (maybe some endpoints don't require authentication), it will be using LoginAdapter. I would instead opt to modify the request itself; take a look at URLRequestConvertible in the docs.
struct LoginApi {
// try to replace Strings with types; you avoid typos and get autocomplete
enum GrantType: String {
case password
}
enum Scope: String {
// ...
}
// ...
static func login(password: String, username: String, ... ) -> URLRequestConvertible {
// I generally avoid force unwrapped optionals, but you get the idea
let url = URL(string: "https://example.com/login")!
var urlRequest = URLRequest(url: url)
urlRequest.httpMethod = HTTPMethod.post.rawValue
// stuff ...
urlRequest.setValue("Basic xxxxxxxxxxxxxxxxx" , forHTTPHeaderField: "Authorization")
urlRequest.setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type")
return urlRequest
}
// ...
}Sent with GitHawk
Got your point @mgacy. So, here we have one more simple way to proceed:
In the Router's asURLRequest() function we can add those extra headers for login api.
public func asURLRequest() throws -> URLRequest {
let url = try ApiConst.baseUrl.asURL()
var urlRequest = URLRequest(url: url.appendingPathComponent(path))
urlRequest.httpMethod = method.rawValue
switch self {
case .login:
urlRequest.setValue("Basic xxxxxxxxxxxxxxx", forHTTPHeaderField: "Authorization")
urlRequest = try URLEncoding.default.encode(urlRequest, with: parameters)
default:
break
}
return urlRequest
}