Sistema de Autorização

filipedeschamps commented 3 years ago

Descrição

Eu como desenvolvedor do sistema, gostaria de poder ter um sistema de autorização que controlasse o acesso aos recursos. O sistema não deveria ser Role-based Access Control (RBAC) e sim ser Resource-based Access Control. Então ao invés de um usuário ter o papel (role) de admin ou moderator que agrega uma variedade de permissões, no caso de controle de acesso por recurso o usuário possui um controle muito mais refinado, por exemplo, qualquer usuário pode receber a permissão para rodar as migrations, ou deletar posts, fazer upload de imagens, criar anúncios ou o que seja de funcionalidade no sistema.

Execução

A evolução de um usuário dentro do TabNews (no quesito de o que ele pode fazer) vai se basear muito em dois pilares: Ações e Recursos. Inicialmente um usuário vai poder apenas criar comentários, por exemplo. Depois de um tempo, ele vai poder criar notícias na home, e depois criar comentários usando formatação em markdown, depois enviar imagens... ou o que seja. Tudo isso é uma combinação de ação contra um recurso. Você pode ler mais sobre isso nesse artigo: The New RBAC: Resource-Based Access Control. Tem um outro tipo de método que é Attribute Based Access Control (ABAC), que parece ser bem standard, assim como o RBAC, mas traz uma granularidade ainda maior, só que eu acho desnecessário para o nosso contexto. De qualquer forma, como sempre, tudo está aberto a discussão 🤝

Sobre a implementação de fato, não vou me prolongar aqui porque só vou conseguir visualizar algo concreto implementando, mas a primeira direção que vou tomar para testar deve ir nessa linha:

// só um rabisco de uma rota da API

import routes from "next-connect";
import authentication from "models/authentication.js";
import authorization from "models/authorization.js";

export default routes()
  .use(authentication())
  .use((req, res) => {
    const user = req.user;
    const resource = req.query.resource;
    const action = req.method;

    if (!authorization.can(user, action, resource) {
      throw new Error('Acesso negado');
    }
  })

Esse rascunho ta bem bagunçado, middleware dentro de models, o nome das coisas de forma estranha, mas independente disso, como falei preciso por a mão no código para visualizar melhor as moving parts envolvidas 👍

No mais, alterações em:

Model de Autorização
Modificar Controllers que precisam de autorização
Modificar User para ter um array de permissões
Testes

Dependência

106

huogerac commented 3 years ago

@filipedeschamps enviei um exemplo rodando para você dar uma olhada, mesmo tem uma stack bem diferente...talvez tenha algo útil! Para não deixar o projeto sair do trilho, deixei privado, se e somente se você ver algo útil ou possa agregar para o andamento do tabnews. Enfim se puder ver: https://github.com/huogerac/tabnews-web/invitations Valeu

filipedeschamps commented 2 years ago

@huogerac nossa eu só vi essa resposta agora que voltei na issue para comentar algo :/ cliquei no invite mas ele expirou, se ainda estiver disponível gostaria total de dar uma olhada 🤝

filipedeschamps commented 2 years ago

Estou precisando de ajuda na parte de Autorização, pois não consegui ainda pensar em uma abstração que faça duas coisas (na verdade não sei se ela deveria fazer duas coisas, mas elas estão interligadas). Essas duas coisas são:

Entender se o usuário tem permissão/feature para uma ação em um objeto (ou propriedade de um objeto)
Retornar propriedades conforme nível de acesso/feature

Sobre o Ponto 1

Implementar isso sem estar abstraído num componente não é muito complicado, até colocando dentro de um componente não é complicado, mas não consegui fazer de uma forma que fique sofisticada, principalmente tentando juntar com o Ponto 2.

Sobre o Ponto 2

Por exemplo: um usuário pode pedir para ver seu objeto de perfil e com isso retornar o seu próprio email (ele pode ver seu próprio email). Agora um outro usuário pedindo o mesmo objeto pode ver as informações menos o email.

Abstração

As únicas soluções que eu encontrei ou implementavam com arquivos de configuração muito bizonhos, ou era tudo espalhado no controller. Então não dá para fazer algo tão simples quanto:

...
  .use(authorizationHandler)
  .get(getHandler)
  .post(postHandler);

Primeiro porque a Autorização não vai ser compartilhada entre o POST e GET, fora que muitas vezes você precisa do objeto retornado dentro do getHandler para tomar alguma decisão (por exemplo ver se o objeto de usuário que está sendo requisitado é do usuário autenticado), mas daí já passou da camada de Autorização.

Eu vou continuar rabiscando aqui sem ter abstraído o código para entender se consigo visualizar as abstrações certas. Mas qualquer sugestão ou experiência nisso é super bem vinda 🤝

rodrigoKulb commented 2 years ago

Realmente a autorização é total regra de negócio, se for pensar em Orientação Objeto a responsabilidade fica com a class Usuário de controle de quem tem acesso.

Podemos criar categorias de usuários exemplo:

Administrador (ver todos usuários e editar)
Membro ( editar somente os próprios dados)

E dentro de cada categoria definir as permissões.

Achei esse vídeo da Dani que fala exatamente a estrutura de permissão e regra, achei bem completo: https://www.youtube.com/watch?v=TGCwB9oMR0o

Basicamente precisamos criar 2 tabelas :

Permission
Role

filipedeschamps commented 2 years ago

Único problema de autorização baseada em Role na minha opinião é que você espera que um usuário não tenha dois Roles simultâneos no sistema. Então isso dificulta um pouco a parte de features do site, de por exemplo:

Inicialmente usuário pode comentar (mas não pode criar posts).
Depois também ganha permissão de criar posts.
Depois também ganha permissão de fazer upload de imagens.
Etc...

E assim vai indo para qualquer outra feature, por exemplo, dar permissão de um usuário que contribui aqui no código do TabNews para ele poder rodar as migrations, mas sem ser Admin, e isso também não interferir no acesso a outras features.

Mas de qualquer forma sensacional a sugestão do vídeo @rodrigoKulb e vou ver com certeza 👍 👍 👍

tcarreira commented 2 years ago

Só deixando um comentário, que eu acho que vc tem presente, mas talvez não seja óbvio para os outros.

Mesmo que a utilização do sistema seja adicionando "roles" e dando umas permissões mais granulares para "resources", para que isso funcione bem, temos de abstrair esse conceito em "actions" e "resources". Ou seja, vc pode continuar pensando em Administradores e Membros, mas tendo uma implementação de RBAC pura.

((ainda) não sei como fazer bonito 😅 )

tcarreira commented 2 years ago

E já agora, para eu que não entendo assim tanto de JS, qual é a limitação de usar este pacote? https://www.npmjs.com/package/rbac

huogerac commented 2 years ago

@huogerac nossa eu só vi essa resposta agora que voltei na issue para comentar algo :/ cliquei no invite mas ele expirou, se ainda estiver disponível gostaria total de dar uma olhada handshake

Hey @filipedeschamps Nossa! achei que tinha ignorado total (o que não tinha problema dado a solução que estava mostrando...hehehe) Bom, vou precisar subir tudo novamente e te passo um novo link/invite! Deixei privado, pq o tabnews é privado e queria respeitar isto!

huogerac commented 2 years ago

Hey @filipedeschamps, fiz o invite novamente: https://github.com/huogerac/tabnews-web/invitations e coloquei no ar a authenticacao utilizando o OAUTH do Github, ou seja, quais seriam as vantagens/desvantagem da versao incial utilizar apenas o login via Github? nao teriamos que ficar lidando com envio de email, reset de senha etc.... E claro, alguem sem Github nao conseguiria criar conta, mas para a versao inicial, talvez isto nao seria um problema...

Pessoal, nao deixei o repo publico, principalmente pq o tabnews esta privado e tambem pq ele esta utilizando algumas tecnologias (como JWT) que ja foi decidido nao utilizar (e que faz sentido)...eu so utilizei pq foi mais simples para este POC para validar o Autorizacao via recursos (Resource-Based Access Control)

p.s: As permissoes dentro de um token (pode comentar e criar noticas, mas nao pode usar markdown nem imagens) fica muito legal, a propria camada da API valida isto tudo, mas em teoria podemos ter o melhor dos 2 mundos, ou seja, usar o modelo "mais seguro" via sessoes para autenticar e ter a autorizacao via token tambem, certo? talvez isto seja possivel, nao acham?

p.s.s.: desculpa a falta de acentos.

ItanuRomero commented 2 years ago

Fala pessoal, estou lendo os comentários nessa issue e tenho uma sugestão: É bem claro que o @filipedeschamps quer evitar roles de usuário, então pensando em encontrar uma estrutura que permitisse o usuário a ir ranqueando seu nível de acesso como dito na seção 8 do diário de desenvolvimento, poderia ser possível a criação de algo semelhante ao usado no sistema Salesforce, as Permission Sets.

Digamos que um usuário acabou de chegar na plataforma, ele pode ser ranqueado como anônimo, pode ver apenas as notícias mas não os comentários e para subir de ranking ele deve logar no sistema. A partir desse momento ele pode ver as notícias e os comentários, por exemplo, e a partir de um certo tempo poderá fazer comentários, criar notícias, e por aí vai. Seguindo nessa ideia, podemos criar grupos de permissões que acompanham o usuário e vão sendo somadas através de sua jornada pelo portal, como por exemplo: Anônimo > Leitor > Comentarista > Escritor > Mantenedor > Administrador

Nesse approach o Leitor possui as permissões de anônimo + likes e visualizar comentários, já o comentarista possui tudo dos anteriores + poder criar comentários, ou algo nesse modo.

Outro ponto a se considerar: O usuário, independente de seu nível de permissão, deve estar habilitado a editar e remover apenas conteúdo próprio, e remover itens não deveria fazer parte das listas de permissão abaixo de Mantenedores ou Moderadores...

Enfim, o que acham que daria certo e errado nessa abordagem? Isso faz sentido ou não? É minha primeira contribuição aqui então se eu estiver falando besteira eu super vou entender os feedbacks :)

tcarreira commented 2 years ago

@ItanuRomero Antes de mais, bem vindo às contribuições :)

Eu entendi que o que vc descreveu é um modelo de roles, em que cada usuário possui um conjunto de permissões. Entendi correto?

Esse modelo funciona bem para estruturas simples, em que existe uma escada clara de roles (existe uma ordem), e o role seguinte tem mais permissões que o anterior.

O problema é que esse modelo não escala para situações mais complexas.
Por exemplo, imaginemos que existem dois usuários, ambos com role de Escritor, mas eles não podem escrever em todos os tópicos (inventei este requisito, só para o exemplo):

Escritor1 só pode publicar nos Tópicos A e B
Escritor2 só pode publicar nos Tópicos B, C e D

Neste tipo de cenários, um esquema de permissões role based não funciona bem (mas há montes de gambiarras em cima do modelo, o wordpress tem muitos exemplos), enquanto um esquema RBAC tem isto na base:

User1: {writer: {TopicA,TopicB}}
User2: {writer: {TopicB,TopicC,TopicD}}

Faz sentido o exemplo? Ajudou?

ItanuRomero commented 2 years ago

@tcarreira brigadão pelas boas vindas!

Quanto ao exemplo, faz todo o sentido, não tenho muita experiência com RBAC, mas me interessei no assunto!

Vou ficar ligado por aqui e ver se consigo dar uma estudada no tema pra poder ajudar

rodrigoKulb commented 2 years ago

Bom dia pessoal!

Nossa que conversa legal, quando eu falo que esse projeto é melhor que qualquer 'curso" ninguém acredita! 🚀️

@ItanuRomero achei sensacional o sistema de permissão em escala Anônimo > Leitor > Comentarista > Escritor > Mantenedor > Administrador. Quem trabalha dessa forma é o Stack Overflow. Gosto muito dessa linha!

@tcarreira realmente existe a limitação de definir permissão por postagem, mas se misturar a hierarquia no exemplo do @ItanuRomero onde somente pessoas com nível acima pode editar postagens de usuários abaixo dela?

Não consigo ver alguma necessidade de definir uma postagem especifica para somente alguns usuários. E como ficaria a gestão dessas permissões?

tcarreira commented 2 years ago

@tcarreira realmente existe a limitação de definir permissão por postagem, mas se misturar a hierarquia no exemplo do @ItanuRomero onde somente pessoas com nível acima pode editar postagens de usuários abaixo dela?

Realmente não é fácil enxergar exemplos quando o projeto ainda está tão abstrato. Mas por exemplo, digamos que vamos ter mantenedores que não podem ser escrever artigos? Como se pode mapear isso apenas com roles, e não ficar uma confusão?

Mas podíamos continuar o projeto só com roles? Com os requisitos atuais, não vejo problema. Será que o melhor caminho é fazer o mais simples e refatorar no futuro?

Pessoalmente tendo a querer implementar coisas mais robustas no início, mas isso tem um preço, que é aumento de complexidade e tempo.

@filipedeschamps vc consegue dar mais contextos concretos de onde um role based não é suficiente?

ItanuRomero commented 2 years ago

Fala pessoal!

Estudei um pouco sobre o tema, principalmente lendo o artigo que o @filipedeschamps indicou na issue, e pensei numa abordagem que seria a seguinte:

Em vez de roles, adicionar cada uma das permissões no objeto do usuário, pensemos assim, usuário é criado com um nome e com um objeto permissões vazio:

const user = {name: 'doesnt matter', permissions: {}}

Esse usuário não possui nenhuma permissão, vamos popular o objeto permissions dele, primeiro criando uma chave de leitura que possui como valor um array vazio:

user.permissions.read = []

Inicializamos o usuário como novo membro do site, e damos a ele o acesso de leitura para posts:

user.permissions.read.push('posts')

Agora, as permissões de leitura do usuário possuem 'posts', show, vamos criar os objetos de post e comments então:

const post = {title: 'doesnt matter', neededPermission: {read: 'posts'}}
const comment = {title: 'doesnt matter', neededPermission: {read: 'comments'}}

Então, agora para saber se um usuário tem acesso de leitura a um post usaremos:

const userPermittedToReadPost = user.permissions.read.includes(post.neededPermission.read)

que retorna true

E o mesmo para comentários:

const userPermittedToReadComment = user.permissions.read.includes(comment.neededPermission.read)

que retorna false

Agora, o objeto comentário pode ter uma função que recebe o objeto de usuário e procura se ele possui acesso para leitura, edição ou remoção.

Sei que a abordagem acima está bem simplificada e que deveria ser muito refatorada, mas quero saber se isso que escrevi acima se aproxima ou se afasta do que queremos fazer aqui, o que acham?

Teoricamente, segue a ideia de que:

O comentário em si possui a regra de negócio que define se o usuário pode vê-lo ou não
O usuário deve receber novas permissões e isso pode ocorrer com o sistema em produção
Até podemos criar um sistema de role, e esses roles são responsáveis por adicionar as permissões únicas no usuário, mas isso agora não implica em problemas para um usuário com 2 roles de uma só vez.

Pra finalizar, a implementação do rabisco que o Filipe colocou acima (if (!authorization.can(user, action, resource)) ficaria assim:

function can(user, action, resource) {
    return user.permissions[action].includes(resource.neededPermission[action])
}

Faz sentido?

huogerac commented 2 years ago

Fala pessoal,

Primeiramente, desculpa que não estou indo ao nível de código...só em fluxo de execução

1) Tentando responder o @tcarreira & pq talvez o modelo de Role Based pode dar mais trabalho

Eu entendi um pouco sua explicação, mas o conceito de "Tópico" não sei se entendi corretamente, vou "tentar" colocar o exemplo do tabnews:

Imagine que de um lado temos as Roles:

// Role e os recursos (endpoint da api) com acesso
Visitante aka sem login: ['tabnews:list']
Escritor-Inicial: ['tabnews:list', 'comments:create']
Escritor-Intermidiario: ['tabnews:list','comments:create', 'tabnews:create']
Escritor-Premium: ['tabnews:list','comments:create', 'tabnews:create', 'image:upload']
Escritor-Mantenedor: ['tabnews:list','comments:create', 'tabnews:create', 'image:upload', 'tabnews:delete', 'comments:delete']

Desta forma, um usuário novo começa como 'Escritor-Inicial', depois de atingir certo número de pontos (fazendo comentários), ele passa para uma nova role 'Intermediario' e assim por diante...

Então, acredito que sim o modelo de Role Based pode ser adaptado e conseguir atender os requisitos, mas talvez pode ter formas mais interessantes ou menos trabalhosa... Imagine ter que ficar criando várias roles, Escritor-Premium2-q-tb-pode-deletar, saca sempre vai ter novas regras e exceções das regras, que complica a criação de Roles.

2) Permissões baseada em Recursos

Primeiramente, não importa se a 'Autenticação' é via login usando OAUTH, usuário e senha ou passwordless Vai ganhar um acesso que pode ser um sessionID ou um Token (tanto faz), só diz que este usuário foi autenticado..

Dai temos os recursos protegidos ou relacionados pela API:

GET /api/tabnews         security: [tabnews:list]
POST /api/tabnews         security: [tabnews:create]
POST /api/comments         security: [comments:create]
POST /api/tabnews/image         security: [image:upload]
...

Assim, imagine que após o login (novamente independente da forma de autenticacao), o usuário vai ganhar uma sessão a qual é ligada a um usuário (USER_ID: 100)

Dai toda vez que ele fizer uma requisição, no lado do backend, vamos:

Pegar o usuário da sessão
Descubrir sua pontuação que vai gerar a lista de recursos com acesso.

Assim vamos ter 2 possibilidades:

Está tentando fazer uma requisição que ainda não ter permissão (exemplo: só pode inserir comentário, mas está tentando inserir tabnews)
Está tetando fazer uma requisição que é permitida, dado que sua pontuação retornou uma lista de recursos que contém a security da requisição

Assim ao inves de uma tabela de Role & permissions Vamos ter uma tabela de ENDPOINT (ou service) vs permission Exemplo: POST /api/tabnews/image security: [image:upload]

assim a implementação da camada de serviços, pode ter um middleware que sabe: Obter o usuário da sessão, calcular seus pontos que por sua vez irá retornar a lista de recursos que ele tem permissão.

Ahhh, mas se o usuário pode fazer upload de imagem, mas só para o tabnews criado por ele (que é o caso) Esta segunda regra, vai estar dentro do serviço, ele passou na primeira validação (middleware) que o permite fazer upload de imagem, dai o serviço específico de upload de imagem vai validar se ele é o dono da tabnews para concluir a requisição com sucesso, caso contrário vai retornar um 403

3) Implementação POC

Nos links que passei acima, tem uma implementação bem inicial desta forma de validação, infelizmente não conheço React e NextJS o suficiente e fiz usando Python para validar o conceito!

Eu acabei utilizando JWT (@filipedeschamps não fique bravo), porque esta tecnlogia me parece que ataca exatamente este problema (Resource-based Access Control) e com pouco código, todo este controle está resolvido. [1] e [2]

Mas tranquilamente é possível tirar o JWT da jogada e manter o mesmo comportamento ou quem sabe pegar o melhor dos 2 mundos: o modelo tradicional de sessão usando token apenas para centralizar o middleware

Ufa, ficou um pouco longo, espero não ter confundido mais pessoal

[1] https://tabnewsapi.herokuapp.com/openapi.yaml

[2] https://tabnewsapi.herokuapp.com/api/

filipedeschamps commented 2 years ago

Turma que thread SENSACIONAL essa daqui se tornou 😍 🤝

@huogerac muito obrigado pelo invite acabei de aceitar e vou com certeza dar uma olhada na POC.

Em paralelo, vou tentar gravar um vídeo com a implementação que fiz e que pode esclarecer alguns pontos dessa discussão e tenho certeza que vai abrir margem para mais discussão 🤝

filipedeschamps commented 2 years ago

Turma, ta aqui o vídeo: https://youtu.be/U7a85J8oSmw 😍

Gravei na paulera, ve se dá para entender 🤝

tcarreira commented 2 years ago

Turma, ta aqui o vídeo: https://youtu.be/U7a85J8oSmw 😍

Fantástico que vc resolveu implementar e explicar tudo com o vídeo. Sensacional! Acho que este tipo de permissões ficam mais fáceis de gerir a médio prazo. E como vc disse, mesmo que na cabeça fique aquela ideia de role, isso basicamente é apenas um conjunto de permissões de template.

Só acho que tem um bug que quando vc voltar a abrir novamente o link de ativação, vai receber o 'usuário não pode ler token' em vez do 'usuário já está ativo' (minuto 16:38) . Porque vc remove as permissões de ler token após ativação.

Deixo ainda uma pergunta. Será que não existe algum pacote externo que implementa aquela validação? Se não existe, acho que faria sentido criar a funcionalidade como um pacote separado (talvez um pouco mais à frente no tempo)

Parabéns 🎉

ItanuRomero commented 2 years ago

Rapaz, sensacional, que experiência. Muito bom o vídeo e a implementação, mas estou me sentindo que nem o Filipe lendo pela primeira vez o artigo do menino que hackeou o Github.

No mais, acho que está bem explicado, consegui entender boa parte mesmo nos meus limites técnicos!

@tcarreira No minuto 19:23 ele mostra a mensagem que aparece pro usuário e ela fala que o usuário pode já estar ativo, mas pode ser uma boa revisar se faz sentido aquela mensagem super técnica para o usuário final.

Valeu pessoal!

filipedeschamps commented 2 years ago

Só acho que tem um bug que quando vc voltar a abrir novamente o link de ativação, vai receber o 'usuário não pode ler token' em vez do 'usuário já está ativo' (minuto 16:38) . Porque vc remove as permissões de ler token após ativação.

@tcarreira seguindo o que o @ItanuRomero colocou na resposta dele, esse é o comportamento esperado porque alguém não pode ativar a conta duas vezes, mas concordo que a mensagem está muito técnica. O código está assim:

  if (!authorization.can(userToActivate, 'read:activation_token')) {
    throw new ForbiddenError({
      message: `O usuário "${userToActivate.username}" não pode ler o token de ativação.`,
      action:
        'Verifique se você está tentando ativar o usuário correto, se ele possui a feature "read:activation_token", ou se ele já está ativo.',
      stack: new Error().stack,
    });
  }

O message diz o que aconteceu, e o action é uma sugestão do que fazer, mas lembro agora, ta um pouco embaralhado mesmo 😂

tcarreira commented 2 years ago

Só acho que tem um bug que quando vc voltar a abrir novamente o link de ativação, vai receber o 'usuário não pode ler token' em vez do 'usuário já está ativo' (minuto 16:38) . Porque vc remove as permissões de ler token após ativação.

@tcarreira seguindo o que o @ItanuRomero colocou na resposta dele, esse é o comportamento esperado porque alguém não pode ativar a conta duas vezes, mas concordo que a mensagem está muito técnica. O código está assim:
  if (!authorization.can(userToActivate, 'read:activation_token')) {
    throw new ForbiddenError({
      message: `O usuário "${userToActivate.username}" não pode ler o token de ativação.`,
      action:
        'Verifique se você está tentando ativar o usuário correto, se ele possui a feature "read:activation_token", ou se ele já está ativo.',
      stack: new Error().stack,
    });
  }
O message diz o que aconteceu, e o action é uma sugestão do que fazer, mas lembro agora, ta um pouco embaralhado mesmo 😂

Não é esse if, é o if seguinte :D Acho que são mutuamente exclusivos (devido a regras de negócio), então o segundo if nunca é alcançado :)

Mas não devia ter falado disso aqui 😅 Estou tirando foco do fantástico trabalho que foi feito. Bug ou não, é fácil mudar depois.

filipedeschamps commented 2 years ago

Não é esse if, é o if seguinte :D

Ahhh interessantíssimo, você está certo @tcarreira ! Vou colocar as informações aqui para todo mundo estar no mesmo barco, esse é o if em questão:

  if (authorization.can(userToActivate, 'create:session')) {
    throw new ForbiddenError({
      message: `O usuário "${userToActivate.username}" já está ativo.`,
      action: 'Você já pode fazer login por possuir a feature "create:session".',
      stack: new Error().stack,
    });
  }

E juntando tudo, a sequência é essa:

// GET /api/v1/activate/:token

if (!authorization.can(userToActivate, 'read:activation_token')) {
    throw...
}

if (authorization.can(userToActivate, 'create:session')) {
    throw...
}

Os fluxos possíveis:

1) Usuário com conta que acabou de criar recebe o link de ativação, clica nele e não vai entrar na primeira condição nem na segunda, pois ele possui somente a feature read:activation_token. 2) Com o token usado, o usuário agora não possui mais read:activation_token. Já aí ele vai ser barrado na primeira condição (vai entrar nela), pois ele não pode mais ler tokens de ativação. Não importa se ele tem ou não a feature create:session. 3) Não consigo mais imaginar (como eu imaginei no momento da implementação) que o usuário possa ter a feature read:activation_token ao mesmo tempo que create:session. A não ser por manipulação manual dos dados das features.

Correto?

filipedeschamps commented 2 years ago

Em paralelo, o que acham de mudarmos a nomenclatura de create para write? Acho que faz um par melhor entre read e write e combina mais com permissões de banco de dados e sistemas operacionais. O raciocínio é que a pessoa tem a permissão de "escrever" em tal local/objeto e casaria melhor num ato de "update", onde não é "create" e sim "write".

👍 - se faz sentido 👎 - se não faz sentido

rodrigoKulb commented 2 years ago

Rapaz, sensacional, que experiência. Muito bom o vídeo e a implementação, mas estou me sentindo que nem o Filipe lendo pela primeira vez o artigo do menino que hackeou o Github.

@ItanuRomero totalmente, agora que já assisti 3 vezes o vídeo ficou um pouco mais claro!

Eu nunca tinha visto esse tipo de permissão, achei incrível trabalhar a permissão user a user, mas aqui pensando não consegui enxergar como você daria uma permissão "inicial" para o adm do site? Isso ficaria em outra flag?

@filipedeschamps se você lançar um curso, eu já estou inscrito!!! hahaha!!

filipedeschamps commented 2 years ago

não consegui enxergar como você daria uma permissão "inicial" para o adm do site? Isso ficaria em outra flag?

Excelente pergunta e hoje não tem como e seria só através de marretar diretamente o banco de dado 😂 mas mais pra frente dá para fazer um fluxo de "setup" e que identifica se não há nenhum usuário com certas features necessárias para moderar 👍

tcarreira commented 2 years ago

Em paralelo, o que acham de mudarmos a nomenclatura de create para write? Acho que faz um par melhor entre read e write e combina mais com permissões de banco de dados e sistemas operacionais. O raciocínio é que a pessoa tem a permissão de "escrever" em tal local/objeto e casaria melhor num ato de "update", onde não é "create" e sim "write".

👍 - se faz sentido 👎 - se não faz sentido

Se me permite acrescentar, acho que as permissões normais (e padrão) são crud:

create
read
update
delete

Esse write é um equivalente de todas as opções de crud?

Acho boa ideia manter um padrão, para que não haja dúvidas mais a frente

filipedeschamps commented 2 years ago

Hmmm interessantíssimo @tcarreira . O write seria acesso de escrita (create / update / delete), não contempla o read.

Mas como estamos 100% em REST, faz sentido estar colado nisso também. Quando eu refatorar o código para commitar eu vou levar tudo isso em consideração 🤝

Essa thread ta MUITO massa 😍

gabrielew commented 2 years ago

Após ver o vídeo/código vi que está sendo utilizado as permissões hard coded em vários arquivos, não seria melhor ter isso em um enum/const? "Levando em consideração a qualidade/refatoração do código no futuro" ex: Centralizar a informação em apenas um lugar. const readPermissions = { ACTIVATION_TOKEN: 'read:activation_token', ... }

filipedeschamps commented 2 years ago

@gabrielew excelente sugestão!!! Poderíamos centralizar tudo isso no componente de authorization mesmo e exportar de lá para quem precisar usar. Por exemplo:

import authentication from `models/authentication.js
...
console.log(authentication.permissions.read.activation_token)
// -> "read:activation_token"

O que penalizaria um pouco o tamanho do código, usando como exemplo o if discutido acima:

if (!authorization.can(userToActivate, authentication.permissions.read.activation_token)) {
    throw...
}

Ao invés de:

if (!authorization.can(userToActivate, 'read:activation_token')) {
    throw...
}

Trade off que identifiquei:

A favor: não cometer errors simples de achar que está passando uma string de feature correta, e na verdade ela tem um typo. Isso também pode acontecer com o objeto e ele pode silenciosamente retornar um undefined (mas dai o authorization.can() vai dar throw.
Contra: Existe a chance de refatorar a string do token, sem refatorar o caminho do objeto? Se não existir, será que isso vai na prática economizar trabalho?

O que a experiência de vocês diz nesse caso?

tcarreira commented 2 years ago

vou escrever uns comentários e deixar um exemplo de código em baixo

O que penalizaria um pouco o tamanho do código, usando como exemplo o if discutido acima:
if (!authorization.can(userToActivate, authentication.permissions.read.activation_token)) {
    throw...
}

Acho que podemos usar alias para o código ficar mais curto, ou o authentication.js exportar um alias mais curto (perms). Não acho que seja realmente um problema

A favor: não cometer errors simples de achar que está passando uma string de feature correta, e na verdade ela tem um typo. Isso também pode acontecer com o objeto

A meu ver, esta é a principal vantagem. Não acho que a refatorização seja realmente uma vantagem neste caso. Existe sempre a opção replace all, caso seja necessário refatorar a string.

Mas a nota que quero deixar é que no caso de haver um arquivo com todas as permissões, faz mais sentido renomear elas para resource.action, para que as permissões fiquem ordenadas por contexto. (activation_token.read)

E até pode ficar facilitado criar aquelas ações de crud automaticamente com uma helper_function.

e ele pode silenciosamente retornar um undefined

Acredito que static code analysis encontra esses erros.

Este é o meu exemplo:

// file: authentication.js

// helper function
function new_permissions(name, other = undefined) {
  perms = {};
  perms[name] = {
    create: name + ":create",
    read: name + ":read",
    update: name + ":update",
    delete: name + ":delete",
  };

  // Handle additional permissions, if any
  if (other !== undefined) {
    other.forEach((p) => {
      perms[name][p] = name + ":" + p;
    });
  }

  return perms;
}

authentication = {
  permissions: {
    ...new_permissions("post"),
    ...new_permissions("activation_token"),
    ...new_permissions("custom", ["read_all", "weird_permission"]),
  },
};

// alias, for shorter code
perms = authentication.permissions;

module.exports = { authentication, perms };

que pode ser usado com

import perms from `models/authentication.js`

// if (!authorization.can(userToActivate, 'read:activation_token')) {
if (!authorization.can(userToActivate, perms.activation_token.read)) {
    throw...
}

o que vcs acham?

filipedeschamps commented 2 years ago

Sensacional @tcarreira muito bem bolado 🤝

Minha antena só ativa pelo fato de acabar criando permissões "ahead of time", no caso do template padrão ali resultante do new_permissions() ser o CRUD inteiro, ou qualquer que seja o padrão para facilitar a criação das permissões. Eu ainda seria cabeça dura e escreveria manualmente permissão a permissão, para ficar explícito o que existe e o que está sendo adicionado/alterado, de forma granular mesmo. Por que por exemplo, sem percebermos, as vezes uma refatoração nessa função possa criar permissões não esperadas, ou condicionais que começam a quebrar a abstração, como uma feature ter permissão só para read, ou uma feature sem CRUD tradicional e somente other para fazermos uma feature flag, por exemplo.

Mas partindo desse seu modelo consegui pensar em outro, e ao invés de exportarmos um objeto com permissões, a gente pode simplesmente validar a string que é enviada... de forma tradicional mesmo.

Então pra atender a minha preocupação anterior de não criar nada ahead of time, podemos construir um objeto (ou array) de actions aceitas, e quando a string de uma action for enviada, por exemplo authorization.can(user, 'read:activation_token') e ela não existir nessa lista, pro-ativamente fazer o componente dar throw num AuthorizationError ou ValidationError explicando que essa action não existe e isso pode ajudar o desenvolvedor a debuggar.

Assim temos:

Interface pública simples.
Mesmo trabalho de refatoração (replace all de strings, sempre muito fácil).
Garantia contra uma action inexistente.
Granularidade para criar actions exatas ao que o sistema está pedindo.
E por conta dessa granularidade, conseguir criar feature flags.

To doido? Confere isso porque eu acordei 3h da manhã por conta do Oliver e não consegui dormir até agora (são 5:40 aqui agora) 😂

filipedeschamps commented 2 years ago

Em paralelo, deveríamos sempre chamar de actions ou features? Porque no user é tratado como uma feature, mas no authorization é tratado como action. Vou padronizar tudo para feature para ver como vai ficar 👍

tcarreira commented 2 years ago

Minha antena só ativa pelo fato de acabar criando permissões "ahead of time", no caso do template padrão ali resultante do new_permissions() ser o CRUD inteiro

Aquele helper não é estritamente necessário (no meu código original eu chamava default_permissions), mas opinião pessoal que se houver permissões "normais" que já existem por padrão, fica mais fácil usar elas sem grande preocupação, mas o que vc disse não deixa de ser verdade (ver código mais abaixo). No caso daquele código, podia criar a permissão sem as defaults bem fácil, já que o objeto é um objeto/dicionário normal:

authentication = {
  permissions: {
    ...default_permissions("post"),
    ...default_permissions("activation_token"),
    ...default_permissions("custom", ["read_all", "weird_permission"]),
    custom2: {
      implode: "custom2:implode",
      explode: "custom2:explode",
    },
  },

e se usa igual:

if (!authorization.can(userToActivate, perms.custom2.explode)) {
    throw...
}

Mas concordo que esta forma de fazer as coisas pode não ajudar assim tanto, então vê o código seguinte

Mas partindo desse seu modelo consegui pensar em outro, e ao invés de exportarmos um objeto com permissões, a gente pode simplesmente validar a string que é enviada... de forma tradicional mesmo.

// ahahah gosto demais da opção de ter permissões padrão :D 
function default_permissions_simple(name, other = undefined) {
  let perms = [
    name + ":create",
    name + ":read",
    name + ":update",
    name + ":delete",
  ]

  // Handle additional permissions, if any
  if (other !== undefined) {
    other.forEach((p) => {
      perms.push(name + ":" + p);
    });
  }
  return perms;
}

// Lista de todas as permissões que a app aceita
authentication = {
  simplePermissions: new Set([
    ...default_permissions_simple("activation_token"),
    "custom3:read", // na verdade, as permissões são apenas strings
    "custom3:write",
  ])
};

usando assim

if (!authorization.can(userToActivate, "activation_token:read" )) {
    throw...
}

onde aquele authorization.can deve ser algo simples como já havia sido referido

function can(user, permission_str) {
  if (!authentication.simplePermissions.has(permission_str)){
    throw new Error("non declared permission " + permission_str);
  }
  return user.features.has(permission_str)
},

Assim temos:

Interface pública simples.

Gosto 👍

Mesmo trabalho de refatoração (replace all de strings, sempre muito fácil).

Gosto 👍

Garantia contra uma action inexistente.

Gosto 👍

Granularidade para criar actions exatas ao que o sistema está pedindo.

Gosto 👍

E por conta dessa granularidade, conseguir criar feature flags.

Gosto 👍

To doido? Confere isso porque eu acordei 3h da manhã por conta do Oliver e não consegui dormir até agora (são 5:40 aqui agora) 😂

Eu estava igual, adormecendo o bebé nos braços, no meio da noite, mas pensando em js 😂 . Tudo louco!

ps: vi que tem MR, vou tentar ver, mas não prometo

tcarreira commented 2 years ago

// Lista de todas as permissões que a app aceita
authentication = {
  simplePermissions: new Set([
    ...default_permissions_simple("activation_token"),
    "custom3:read", // na verdade, as permissões são apenas strings
    "custom3:write",
  ])
};

Ah, e estou usando um Set porque é mais eficiente procurar nele do que em um Array

filipedeschamps commented 2 years ago

Show!! Então sugestão: a base das duas soluções está igual e uma adiciona um helper. Eu vou então continuar do modo sem helper (mas trocando para Set como você fez) e vamos deixar o código chamar pela necessidade disso 🤝

E sobre o PR, eu quero só terminar de fazer mais uma implementação que eu comentei lá e acho que ta massa para merge 👍

tcarreira commented 2 years ago

Fantástico. Fico ansiando pelo dia em que podemos agendar um pair programming em real time, estilo Dojo ;)

rodrigoKulb commented 2 years ago

Confesso que fiquei um pouco perdido no final, Vou estudar o código final depois! Aqui é só conversa de alto nível 🤩️

filipedeschamps commented 2 years ago

@rodrigoKulb show! Eu vou fazer um novo vídeo, explicando tudo com mais calma, vai valer a pena 👍

gabrielew commented 2 years ago

Uma dúvida que me surgiu aqui, irá existir templates com certas autorizações pre definidas?

filipedeschamps commented 2 years ago

@gabrielew boa pergunta, por enquanto imagino que não. Na verdade os únicos templates por enquanto são do usuário básico (que no último PR as features vem da migration) e o usuário anônimo que está no código, e que talvez tudo isso poderia ser convertido para template de features 👍

filipedeschamps commented 2 years ago

Pessoal, encerramos a Milestone 2 com o merge desse PR: https://github.com/filipedeschamps/tabnews.com.br/pull/195

Que massa!!!

Agora vou ver se vale a pena refatorar algumas coisas e fazer a live de encerramento! E nessa live eu fecho essa issue aqui e lançamos de fato a nossa primeira versão da API pra valer, criando usuários, etc.. 🤝

tcarreira commented 2 years ago

Acho que esta issue está terminada com o #170

filipedeschamps commented 2 years ago

Eu queria fechar ela na live de encerramento da milestone 😂 👍

filipedeschamps / tabnews.com.br