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

#44 Login implementado #84

Merged
merged 16 commits into from
Sep 30, 2019
Merged

Conversation

RenatoBrittoAraujo
Copy link
Collaborator

@RenatoBrittoAraujo RenatoBrittoAraujo commented Sep 27, 2019

Descrição

Foi implementada a autenticação de usuário (login)

Resolve (Issues)

Issue #44

PRs relacionados

Branch PR
feature/44-user-signin (frontend) link

Tarefas gerais realizadas

  • Criar sessão por meio da api;
  • Criar formulário no frontend;
  • Comunicação de token entre front e back;
  • Envio do formulário do frontend para o backend;
  • Resposta de sucesso ou fracasso enviado do backend para o frontend;
  • Frontend deve processar a resposta do backend, informando o usuário
  • Adicionar botão de login;
  • Adicionar botão de logout;
  • Encerrar sessão por meio da api;

Para executar:

Para testar, execute o backend (branch: feature/44-user-authentication) e frontend (branch: feature/44-user-signin) pelo docker-compose usando suas portas padrões, crie um usuário diretamente pelo backend por esse caminho e depois entre na página '/signin' do frontend e faça a autenticação de usuário usando email e senha. Você será direcionado para a home onde você pode ver os tokens access e refresh e fazer logout ao pressionar o botão.

Página de signin (mobile)
image

Ao ser autenticado com sucesso você será redirecionado para a home e poderá ver os tokens registrados (que estão guardados nos cookies)

image

Durval Carvalho and others added 13 commits September 24, 2019 14:46
Co-authored-by: Leonardo da Silva Gomes <leonardodasigomes@gmail.com>
Co-authored-by: Leonardo da Silva Gomes <leonardodasigomes@gmail.com>
Co-authored-by: Leonardo da Silva Gomes <leonardodasigomes@gmail.com>
Co-authored-by: Renato Britto Araujo <renatomwbbritto@gmail.com>
Co-authored-by: Renato Britto Araujo <renatomwbbritto@gmail.com>
Co-authored-by: Flavio Vieira <flavio.vl@gmail.com>
Co-authored-by: Renato Britto Araujo <renatomwbbritto@gmail.com>
from django.contrib import admin
from .models import User

class UserAdmin(admin.ModelAdmin):
Copy link
Member

Choose a reason for hiding this comment

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

Usando ModelAdmin ele coloca a hash na senha? (Como faz o UserAdmin do django.contrib.auth.admin?

Copy link
Collaborator

@durvalcarvalho durvalcarvalho Sep 27, 2019

Choose a reason for hiding this comment

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

Essa classe User que criamos está usando os mecanismos de senha do user padrão do Django, esses mecanismos vem da herança no AbstractUser, lá no arquivo models.py

class User(AbstractUser):

Essa classe UserAdmin foi criada para podermos acessar o painel do admin do django ( /admin/ ). Já que não estamos usando o User nativo do django é preciso explicitar os campos que irão aparecer no painel.

image

Copy link
Member

Choose a reason for hiding this comment

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

só pra esclarecer, o useradmin é um tipo de modeladmin mais adequado, mas ok

@@ -0,0 +1,50 @@
# Generated by Django 2.2.4 on 2019-09-24 19:30
Copy link
Member

Choose a reason for hiding this comment

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

@vitorcx @shayanealcantara talvez devessemos criar uma issue futura pra gerenciarmos migrações (até então não vai dar problema, mas pode dar no futuro, quando estivermos trabalhando em várias funcinoalidades e branchs, pois elas são geradas sequenciamente e commitá-las pode dar algum problema

Copy link
Collaborator

@durvalcarvalho durvalcarvalho Sep 27, 2019

Choose a reason for hiding this comment

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

Eu pensei em não criar commits em nenhuma migração. Eu acho que enquanto o nosso banco de dados não estiver em produção, não tem problema ficarmos zerando o banco e criando todas as migrações do zero.


EMAIL_FIELD = 'email'
USERNAME_FIELD = 'email'
REQUIRED_FIELDS = ['username']
Copy link
Member

Choose a reason for hiding this comment

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

Username como campo obrigatório, existe algum motivo?

Copy link
Collaborator

@durvalcarvalho durvalcarvalho Sep 27, 2019

Choose a reason for hiding this comment

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

Sim! O Django.

Eu tirei ele durante alguns teste, mas quando tento entrar no painel do admin do django a aplicação quebra. Aparentemente os templates do django admin usam a variável {{user.username}}, e se ela não existir quebra tudo.

Uma solução seria sobrescrever esse template, mas eu não achei que seria uma boa ideia...

Copy link
Member

Choose a reason for hiding this comment

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

teria uma solução mais interessante, mas fica pra outra sprint, criar com lambda, a partir do email do usuário um username

# this fields dont belongs to this class
validated_data.pop('confirm_password')

user = User.objects.create_user(**validated_data,
Copy link
Member

Choose a reason for hiding this comment

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

dessa forma, vocês não tão salvando no banco de dados a senha em claro?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Estamos não, estamos usando os mesmos mecanismos de hash do usuário padrão do django.

Segue alguns prints comprovando o que estou falando.

Criação do Usuário

image

Usuário criado

image

Usuários no django Admin

image

Usuário novo com senha criptografada

image

@codecov-io
Copy link

codecov-io commented Sep 28, 2019

Codecov Report

Merging #84 into develop will decrease coverage by 0.53%.
The diff coverage is 93.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     #84      +/-   ##
==========================================
- Coverage    93.93%   93.4%   -0.54%     
==========================================
  Files            3      11       +8     
  Lines           33     197     +164     
==========================================
+ Hits            31     184     +153     
- Misses           2      13      +11
Flag Coverage Δ
#unittests 93.4% <93.41%> (-0.54%) ⬇️
Impacted Files Coverage Δ
src/users/viewsets.py 100% <100%> (ø)
src/users/urls.py 100% <100%> (ø)
src/users/migrations/0001_initial.py 100% <100%> (ø)
src/users/admin.py 100% <100%> (ø)
src/users/tests.py 100% <100%> (ø)
src/acacia/urls.py 100% <100%> (ø) ⬆️
src/users/models.py 100% <100%> (ø)
src/acacia/settings.py 100% <100%> (ø) ⬆️
src/scripts/wait_for_db.py 79.41% <79.41%> (ø)
src/users/serializers.py 89.74% <89.74%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bb8080...cdcb181. Read the comment docs.

@RenatoBrittoAraujo
Copy link
Collaborator Author

@fabiolamfleury Forams implementados os tests de api no backend, completando a lista de requisitos para este pull request no backend

@fabiolamfleury fabiolamfleury merged commit b8cf370 into develop Sep 30, 2019
@fabiolamfleury fabiolamfleury deleted the feature/44-user-authentication branch September 30, 2019 22:22
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.

5 participants