nfephp-org / sped-common

Classes comuns usadas nas atividades e classes das API's de NFe, CTe e MDFe
Other
97 stars 122 forks source link

Sugestão: alteração do método Signer->sign para deduzir o 'rootname' ao invés de recebê-lo por parâmetro #181

Closed zoiosilva closed 6 years ago

zoiosilva commented 6 years ago

Minha sugestão para melhorar a classe é que, ao invés do método público sign receber o nome do node ($rootname) que receberá a assinatura, o método deduza esta informação através da propriedade $node->parentNode.

Se eu não estiver enganado, o node com a assinatura digital deve sempre vir após o node sendo assinado. Assim, não faz muito sentido esta informação vir de fora, uma vez que ela pode ser facilmente deduzida usando as propriedades da classe DOMElement.

Vide modificação da linha 126.

robmachado commented 6 years ago

@zoiosilva Isso é verdade apenas no caso da NFe, CTe e MDFe em outros casos não é verdade e isso quebraria o código e impossibilitaria seu uso em outros casos como por exemplo o NFSe

tonicospinelli commented 6 years ago

neste caso, quando existe quebra de compatibilidade com código existente, acho mais válido criar um novo método e marcar este como @deprecated justificativa e sugestão de uso do novo método

@robmachado pode exemplificar por que não funciona com o NFSe?

robmachado commented 6 years ago

@tonicospinelli Da forma como está hoje, eu posso usar essa classe para fazer assinaturas em qualquer XML que use sha-1 ou sha-256 Algumas estruturas de XML assinam outras TAG e não a tag logo abaixo da RAIZ, alguns casos assinam a própria raiz do xml, na realidade isso não é nem de longe PADRONIZADO. O que o @zoiosilva propôs simplifica o trabalho do desenvolvedor na hora de chamar a função, mas a deixa muito mais ESPECIFICA e não aproveitável em outros casos. Particularmente inclusive eu gostaria de deixar essa classe mais adaptável e configurável (mais genérica ainda). Eu inclusive tentei usar a biblioteca do Robert Richards XMLSECLIBS mas não atende os padrões da SEFAZ, vale a pena dar uma olhada no que ele fez e aprimorar as nossas estruturas. E quem sabe até deslocar essas classes para outro repositório especifico para certificados, assinaturas e encriptação.

zoiosilva commented 6 years ago

O que me inspirou a fazer esta modificação foi a implementação da NFS-e para a prefeitura de Porto Alegre. Pelo menos para esta prefeitura, funciona da forma como coloquei. A observação do @robmachado é muito boa. Vou dar uma olhada melhor nos outros projetos que usam esta classe com calma mais tarde.

Devo fechar este PR, ou deixar aberto ainda?

robmachado commented 6 years ago

Prezado colega e colaborador Rafael, peço que encerre esse PR até discutirmos mais essa questão. Se desejar pode abrir um ISSUE para esse assunto e vamos manter em pauta. Não quero esquecer.