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

Kalunge/create new user account api endpoint #48

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Kalunge
Copy link
Collaborator

@Kalunge Kalunge commented Mar 10, 2023

This pull request adds a user registration endpoint to the Sinatra application. The endpoint accepts a POST request with JSON data containing the user's email and password. Upon successful registration, a JSON Web Token (JWT) is generated for the user, which can be used to authenticate future requests. The UserController class has been created to handle the /register endpoint. The class includes error handling for missing email and password fields in the JSON data, allows cross-origin requests, and generates a JWT token for the user upon successful registration.

Changes Made

  • Added a new UserController class to handle user registration.
  • Added a new /register endpoint to the application for user registration.
  • Defined a post request handler for the /register endpoint in the UserController class.
  • Extracted the email and password fields from the JSON data passed in the request body.
  • Included error handling to check if the email and password fields are present in the JSON data.
  • Generated a JWT token for the user upon successful registration.
  • Return the generated JWT token in the response in JSON format.

@github-actions
Copy link

github-actions bot commented Mar 10, 2023

Visit the preview URL for this PR (updated for commit 363973c):

https://moringa-library--pr48-kalunge-create-new-u-e30r6l4w.web.app

(expires Thu, 30 Mar 2023 08:59:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 28c4ac8715d663d94559951bf1a9d3f1dcb7979f

@otsembo otsembo linked an issue Mar 10, 2023 that may be closed by this pull request
2 tasks
Copy link
Contributor

@otsembo otsembo left a comment

Choose a reason for hiding this comment

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

Hello @Kalunge, great initial PR.
There are some changes I have requested in the comments.

The database migration files will be ready by tomorrow so that you can use the validation errors that have been added. In the meantime, you can work on any of the comments that do not need the user db models right now.

Comment on lines 1 to 3
require 'sinatra'
require 'jwt'

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are using Rack, we do not need the require statements at the top. Everything will be automatically set up by the bundler. Ensure the gems are listed in the Gemfile and everything should work accordingly.

require 'sinatra'
require 'jwt'

class UserController < Sinatra::Base
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of inheriting Sinatra::Base in every controller, inherit the Base Controller, it will have all the header options and formatting methods by default. You won't need to set them up afresh in every single controller.

require 'jwt'

class UserController < Sinatra::Base
JWT_SECRET = 'my$ecretK3y'
Copy link
Contributor

Choose a reason for hiding this comment

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

Kindly store your JWT_SECRET in an environment variable.
Secrets should not be exposed directly in code.

Comment on lines 7 to 40
# @API: Enable CORS headers from all origins
configure do
enable :cross_origin
end

before do
response.headers['Access-Control-Allow-Origin'] = '*'
end

options '*' do
response.headers['Allow'] = 'GET, PUT, POST, DELETE, OPTIONS'
response.headers['Access-Control-Allow-Headers'] = 'Authorization, Content-Type, Accept, X-User-Email, X-Auth-Token'
response.headers['Access-Control-Allow-Origin'] = '*'
200
end

# @API: Format all JSON responses
def json_response(code: 200, data: nil)
status = [200, 201].include?(code) ? 'SUCCESS' : 'FAILED'
headers['Content-Type'] = 'application/json'
return unless data

[code, { data: data, message: status }.to_json]
end

# @API: Format all common error responses as JSON (thrown errors)
def error_response(err:, code: 422)
json_response(code: code, data: { error: err.message })
end

# @API: 404 handler
not_found do
json_response(code: 404, data: { error: 'You seem lost. That page does not exist!' })
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Once you inherit the BaseController, all these methods are no longer needed. You can safely delete these methods.

Comment on lines 49 to 51
if email.nil? || password.nil?
error_response(err: 'Email and password are required')
end
Copy link
Contributor

Choose a reason for hiding this comment

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

You can omit these checks since you will have been provided with validations on the model already. The validations will have customized error messages already.

Comment on lines 54 to 55
payload = { email: email }
token = JWT.encode payload, JWT_SECRET, 'HS256'
Copy link
Contributor

Choose a reason for hiding this comment

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

You can generate the JWT token after the user is successfully registered. You can move this code after you've created a valid user account.
Also, please make sure you add a timeout for the token. (You can have it set at 6 hours)

Copy link
Contributor

@otsembo otsembo left a comment

Choose a reason for hiding this comment

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

Hello @Kalunge
You have API checks that are failing. Kindly fix those first.

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.

Auth: Create new User account
2 participants