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

adding the implemention of cookies only #63

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hassanrbh
Copy link

… try to write tests and run them, nothing shows

@hassanrbh
Copy link
Author

hassanrbh commented Oct 9, 2022

needs codeclimate to change the limit of Cognitive Complexity

@hassanrbh
Copy link
Author

hassanrbh commented Oct 9, 2022

Tokens Controller

`

     def create 
      create_token_and_set_header(current_resource, resource_name)
      get_token_and_set_cookie
      access_token = get_access_token_from_headers
      remove_access_token_from_headers
      @refresh_token.destroy
      blacklist_token if ApiGuard.blacklist_token_after_refreshing
    
      render_success(data: access_token, message: I18n.t('api_guard.access_token.refreshed'))
    end
private

def get_access_token_from_headers
  response.headers['Access-Token'] if response.headers
end

def get_token_and_set_cookie
  refresh_token = response.headers['Refresh-Token']
  cookies.signed[:jti]={
    :value=>refresh_token,
    :httponly=>true
  }
  remove_keys = %w(Refresh-Token)
  response.headers.delete_if { |key| remove_keys.include?(key)}
end

def remove_access_token_from_headers
  remove_keys = %w(Access-Token Expire-At)
  response.headers.delete_if { |key| remove_keys.include?(key)}
end

def find_refresh_token
  refresh_token_from_header = cookies.signed[:jti]    
  if refresh_token_from_header
    @refresh_token = find_refresh_token_of(current_resource, refresh_token_from_header)
    return render_error(401, message: I18n.t('api_guard.refresh_token.invalid')) unless @refresh_token
  else
    render_error(401, message: I18n.t('api_guard.refresh_token.missing'))
  end
end

`

Registration Controller

def create
      init_resource(sign_up_params)
      if resource.save
        create_token_and_set_header(resource, resource_name)
        get_token_and_set_cookie
        access_token = get_access_token_from_headers
        remove_access_token_from_headers
        render_success(data: access_token,message: I18n.t('api_guard.registration.signed_up'))
      else
        render_error(422, object: resource)
      end
    end

    def destroy
      current_resource.destroy
      render_success(message: I18n.t('api_guard.registration.account_deleted'))
    end

    private

    def get_access_token_from_headers
      response.headers['access-token']
    end

    def remove_access_token_from_headers
      remove_keys = %w(Access-Token Expire-At)
      response.headers.delete_if { |key| remove_keys.include?(key)}
    end

    def get_token_and_set_cookie
      refresh_token = response.headers['Refresh-Token']
      cookies.signed[:jti]={
        :value=>refresh_token,
        :httponly=>true
      }
      remove_keys = %w(Refresh-Token)
      response.headers.delete_if { |key| remove_keys.include?(key)}
    end

Tokens Controller

def create
      create_token_and_set_header(current_resource, resource_name)
      get_token_and_set_cookie
      access_token = get_access_token_from_headers
      remove_access_token_from_headers
      @refresh_token.destroy
      blacklist_token if ApiGuard.blacklist_token_after_refreshing
    
      render_success(data: access_token, message: I18n.t('api_guard.access_token.refreshed'))
    end

    private

    def get_access_token_from_headers
      response.headers['Access-Token'] if response.headers
    end

    def get_token_and_set_cookie
      refresh_token = response.headers['Refresh-Token']
      cookies.signed[:jti]={
        :value=>refresh_token,
        :httponly=>true
      }
      remove_keys = %w(Refresh-Token)
      response.headers.delete_if { |key| remove_keys.include?(key)}
    end

    def remove_access_token_from_headers
      remove_keys = %w(Access-Token Expire-At)
      response.headers.delete_if { |key| remove_keys.include?(key)}
    end

    def find_refresh_token
      refresh_token_from_header = cookies.signed[:jti]    
      if refresh_token_from_header
        @refresh_token = find_refresh_token_of(current_resource, refresh_token_from_header)
        return render_error(401, message: I18n.t('api_guard.refresh_token.invalid')) unless @refresh_token
      else
        render_error(401, message: I18n.t('api_guard.refresh_token.missing'))
      end
    end

@Gokul595
Copy link
Owner

Thanks for the PR. I will take a look in a week.

@hassanrbh
Copy link
Author

cool, let me know. if I did something wrong,

@hassanrbh hassanrbh changed the title adding the implemention of cookies only, but I am not sure why when I… adding the implemention of cookies only Oct 30, 2022
@jrmhaig
Copy link

jrmhaig commented Nov 11, 2022

Thanks for this. I am interested in seeing this feature merged.

Would it be possible, please, to add some notes about the configuration options to README.md and adding them to the initializer template in lib/generators/api_guard/initializer/templates/initializer.rb? I think they are straightforward but being able to find them without reading through the code would be helpful.

@hassanrbh
Copy link
Author

@jrmhaig hey, sorry for not being late in my response, I am applying to some jobs this day so I am busy looking, yeah for sure, I will be free this weekend, I am gonna write some docs about, thanks

@jrmhaig
Copy link

jrmhaig commented Nov 18, 2022

@jrmhaig hey, sorry for not being late in my response, I am applying to some jobs this day so I am busy looking, yeah for sure, I will be free this weekend, I am gonna write some docs about, thanks

No problem. I was really just saying that this would be a great feature to have and thank you for working on it. Good luck in your job applications.

@hassanrbh
Copy link
Author

@jrmhaig thanks dude, love to hear that

Copy link

@Bilanuk Bilanuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR. This gem would be devise killer for APIs. I am doing my uni project with rails and react, and i used the idea of this pull request and also did my fork with cookies implementation. But i just got confused with token refreshing logic - why isn't it possible to refresh access token that already has expired? @Gokul595. I think it would be cool to implement refreshing with only refresh token. Or am i not getting a point of authenticating_resource for refreshing?
Here is fork with example of my idea: Bilanuk@b66920d (still needs a lot of work to be prod ready) If you are interested in this features and my help if needed let me know.
UPD: i made draft PR: #66

@hassanrbh
Copy link
Author

let me give you a flow of production-ready authorization

  1. we create an access token and refresh token ( the purpose of refresh tokens is just for refreshing the access token ( so it's getting a new access token to maintain the session of the user and not having to ask the user to log in again)
  2. we pass the access token just in the response body, and refresh token in the cookies for preventing XSS attacks through any kind of vulnerabilities in our app that gonna lead that attack to us.
  3. the client (react) will accept the request, and set the cookies in the user cookies session ( refresh tokens can expire in 1 year or so, the access token is different, you need to make sure to expire the access token in 1h or 30 min ) and store that access token in a variable in react and persist that or just keep the access token in the session so you just gonna keep it in the server, not the client

and also I get what are you saying but we can refresh them both, the access token and the refresh token in the same query, an example is when we identify a 403 forbidden response we send to the backend the refresh token, we expire the access token and refresh token and get new ones

@hassanrbh
Copy link
Author

and also the purpose of blacklisting the access token is when a user's account gets hacked and that user still has the access token we can just expire that immediately but include that in the blacklist table and also it gives us the power to keep tracking of all the sessions that the user created ( smartphone, laptop, ...)

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

Successfully merging this pull request may close these issues.

4 participants