Ticket #607 (closed defeito: invalid)

Opened 15 years ago

Last modified 14 years ago

Desfazer deterioração de codificação feita por insipiência.

Reported by: rodsouza Owned by: alguem
Priority: média Milestone: Expresso 2.1
Component: API Version: trunk
Severity: Keywords:
Cc: WorkGroup:

Description

Códigos foram deteriorados por insipiência.

Essas alterações foram realizadas provavelmente por codificadores neófitos. Outra provável situação é a incipiência no contexto com isso efetuando alterações imprudentes.

Observa-se ainda que essa deterioração foi realizada por situações contraditórias.

Um exemplo do que está sendo relatado é a retirada do bloqueio de acesso quando o usuário erra a senha.

Outro exemplo é a manutenção da relação 'phpgw_access_log' que foi retirada do código e como consequência se criou a necessidade de criar uma tarefa agendada no sistema operacional para realizar o procedimento.

Existem inúmeros outros exemplos que poderiam ser citados entretando os que foram mencionados demonstram que a deteriorização do software foi realizada por codificadores néscios.

Os exemplos mencionados estão em 'phpgwapi/inc/class.sessions.inc.php'. Vide as operações 'login_blocked' e 'log_access'.

Ainda existe o fato que a deteriorização propiciou a incompatibilidade com a API utilizada além de prover depressões de funcionalidades e de segurança.

Change History

comment:1 follow-up: ↓ 2 Changed 15 years ago by rodsouza

Desfazendo as mudanças para que seja possível identificar os fatos adversos provenientes da alteração realizada no passado e de forma equivocada.

Committed revision r1282.

comment:2 in reply to: ↑ 1 Changed 15 years ago by niltonneto

Replying to rodsouza:

Desfazendo as mudanças para que seja possível identificar os fatos adversos provenientes da alteração realizada no passado e de forma equivocada.

Committed revision r1282.

Rodrigo, essas alterações foram necessárias e já estão há anos em produção. É importante discutirmos antes de fazer qualquer alteração desse impacto
1 - Bloqueio por IP desabilitado: O bloqueio por IP, após "n" tentativas, vai dar problema quando milhares de usuários utilizarem um proxy para acesso ao Expresso. Na época bloqueava a todo momento e por isso forçam essa desativação. Não recomendo reverter essa modificação, a menos que se corrige a configuração na administração do Expresso antes.
2 - Ativação da query que registra a phpgw_access_log: Outro fato ocorrido anos atrás era a lentidão de acesso quando 25.000 usuários logam e deslogam a todo momento. Tá loco, toda vez ele executava a query "DELETE FROM phpgw_access_log WHERE li < $max_age" para cada cada usuário, na login e logout. O banco "sentou" e não aguentou. A maneira mais inteligente foi desabilitar (acredite, o servidor postgresql de produção agradeceu etermnamente!!!!) e criar um rotinar via crontab para que ele limpa essa tabela de tempos em tempos. Também não recomendo reverter essa modificação, a menos que faça o administrador do Expresso instalar essa linha no crontab do Apache, conforme a configuração setada.

comment:3 follow-ups: ↓ 6 ↓ 8 Changed 15 years ago by rodsouza

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

comment:4 follow-up: ↓ 7 Changed 15 years ago by amuller

Já está em produção à anos não é bom argumento. Principalmente se tiver a anos com este problema de não bloquear sucessivos acessos errados. Fica muito fácil quebrar senhas de usuários se não tiver este bloqueio.

E a idéia do crontab é meio parecida com Garbage Collector que é parte da arte de amarrar cachorro com lingüiça.

comment:5 Changed 15 years ago by rodsouza

Outra situação envolvendo sessão é a retirada de cache que existia até um determinado momento e que ultimamente está sendo apontado como a solução para os problemas do mundo.

A justificativa para isso foi a mesma, o base de dados irá ficar sobrecarregada. Se isso fosse fato então não se por que utilizar um serviço que não faz o mínimo esperado?

Se a cache feita foi retirada então por que colocar ela novamente? Assim seguiriamos essa linha de fazer as coisas "deu problema, comenta" ao invés de observar o real motivo.

comment:6 in reply to: ↑ 3 ; follow-up: ↓ 10 Changed 15 years ago by diogenesduarte

Replying to rodsouza:

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

De fato, aqui na PRODEB temos um código um pouco antigo, que ainda está habilitado a questão do bloqueio por muitas tentativas e a exclusão no max_age e tudo está tranquilo, sem problemas de stress no banco. Na verdade havia um problema antes que fazia com que todos os registros de bad login ou password fizessem sofressem um update à cada logoff, e isso gerava um custo enorme para a base depois de um tempo usando o expresso, com uma quantidade grande de registros atendendo essa restrição. Isso já foi corrigido, tá no ticket #455.

comment:7 in reply to: ↑ 4 Changed 15 years ago by niltonneto

Replying to amuller:

Já está em produção à anos não é bom argumento. Principalmente se tiver a anos com este problema de não bloquear sucessivos acessos errados. Fica muito fácil quebrar senhas de usuários se não tiver este bloqueio.

E a idéia do crontab é meio parecida com Garbage Collector que é parte da arte de amarrar cachorro com lingüiça.

Bem, sobre quebrar senhas de usuários no Expresso, sempre existiu lá em Configurações do Servidor a opção de bloqueio do login por "n" tentativas (essa funciona e está ativa), e também bloqueio do IP por "n" tentativas no mesmo IP. Por algum motivo, em produção, deixávamos em branco esse último campo (por IP) e não adiantava. Implementação mal feita desse item? Talvez. Como não tínhamos tempo para depurar, não fomos atrás e acabou ficando assim. Aí acontecia que, se "fulano", "beltrano" erravam a senha uma única vez, contabilizavam 2 tentativas no mesmo IP, já que usavam o mesmo proxy, e tempos depois bloqueava o IP daquela rede.

comment:8 in reply to: ↑ 3 ; follow-up: ↓ 9 Changed 15 years ago by niltonneto

Replying to rodsouza:

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

O que eu citei foram duas situação completamente diferentes e independente uma da outra. Uma é o bloqueio por IP e outra é a execução da query DELETE feita por todos que logam e deslogam (realmente não tem nada ver com IP).

comment:9 in reply to: ↑ 8 Changed 15 years ago by diogenesduarte

Replying to niltonneto:

Replying to rodsouza:

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

O que eu citei foram duas situação completamente diferentes e independente uma da outra. Uma é o bloqueio por IP e outra é a execução da query DELETE feita por todos que logam e deslogam (realmente não tem nada ver com IP).

Verdade, e quanto ao delete acho que ele não tem problema nenhum, mas quanto à questão do IP, Nilton tá certo... Tanto que quando nos deparamos com esse problema aqui, colocamos o parâmetro de tentativas para bloquear por ip para 9999, senão os usuários que passavam pro proxy deveriam ser treinados a não errar a senha, pois o ip de todos é o mesmo, se 3 ou 4 erram a senha uma única vez, bloqueava todo mundo, não só os 3 ou 4, mas todos aqueles que passavam pelo proxy.

comment:10 in reply to: ↑ 6 ; follow-up: ↓ 11 Changed 15 years ago by niltonneto

Replying to diogenesduarte:

Replying to rodsouza:

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

De fato, aqui na PRODEB temos um código um pouco antigo, que ainda está habilitado a questão do bloqueio por muitas tentativas e a exclusão no max_age e tudo está tranquilo, sem problemas de stress no banco. Na verdade havia um problema antes que fazia com que todos os registros de bad login ou password fizessem sofressem um update à cada logoff, e isso gerava um custo enorme para a base depois de um tempo usando o expresso, com uma quantidade grande de registros atendendo essa restrição. Isso já foi corrigido, tá no ticket #455.

Não estou querendo gerar nenhuma polêmica. Apenas atento ao fato de mexer em algo que não foi estudado a fundo, para não se ter efeitos colaterais. Se você tivesse que colocar o Expresso no ar, há cinco anos atrás, e não tivesse como escolher o servidor ideal para prover o serviço PostgreSQL, não diria isso. Não se consegue tirar leite de pedra. Aí o que vc faz? Começar a rever a aplicação, na tentativa de minimizar a carga e encontrar o gargalo. Eu lembro que ligamos o log de queries do banco, e essa query estava entre as mais executadas. Aí desabilitamos ela e automatizamos essa "limpeza" no crontab. Melhorou significativamente o serviço.

comment:11 in reply to: ↑ 10 Changed 15 years ago by diogenesduarte

Replying to niltonneto:

Replying to diogenesduarte:

Replying to rodsouza:

O bloqueio de IP é realizado com uma consulta ao banco que não leva em consideração o usuário.

25.000 registros para uma base de dados não faz nem cócegas. Se essa simples exclusão fez tanta diferença o problema não é lógico e sim de serviço. De forma alguma o um sgbd sofre com instruções simples, nem mesmo os mais simplórios.

A não exclusão ou mesmo ela de forma arbitrária é outro fato que gera o problema do bloqueio de IP.

E sem dizer que esse é apenas um floco da bola de neve que se criou.

Outro fato é que as alterações estão no trunk então será possível identificar inúmeras situações até que a nova versão do ExpressoLivre? seja consolidada.

De fato, aqui na PRODEB temos um código um pouco antigo, que ainda está habilitado a questão do bloqueio por muitas tentativas e a exclusão no max_age e tudo está tranquilo, sem problemas de stress no banco. Na verdade havia um problema antes que fazia com que todos os registros de bad login ou password fizessem sofressem um update à cada logoff, e isso gerava um custo enorme para a base depois de um tempo usando o expresso, com uma quantidade grande de registros atendendo essa restrição. Isso já foi corrigido, tá no ticket #455.

Não estou querendo gerar nenhuma polêmica. Apenas atento ao fato de mexer em algo que não foi estudado a fundo, para não se ter efeitos colaterais. Se você tivesse que colocar o Expresso no ar, há cinco anos atrás, e não tivesse como escolher o servidor ideal para prover o serviço PostgreSQL, não diria isso. Não se consegue tirar leite de pedra. Aí o que vc faz? Começar a rever a aplicação, na tentativa de minimizar a carga e encontrar o gargalo. Eu lembro que ligamos o log de queries do banco, e essa query estava entre as mais executadas. Aí desabilitamos ela e automatizamos essa "limpeza" no crontab. Melhorou significativamente o serviço.

Tem a questão do ambiente também... Aqui pode estar tudo tranquilo, mas em outro ambiente pode ser que atrapalhe mesmo... Não vejo a solução de limpar o log a partir do chron como uma decisão impensada, ou feita sem análise do contexto. Creio que funciona, e se resolveu o problema no ambiente que foi descoberto, não vejo problemas em mantê-la.

comment:12 Changed 15 years ago by amuller

A query em questão só pode ser ruim pois utiliza a clausula heaving por uma coluna não indexada. O que não significa que e inviável. Utilizando o id do usuário como índice hash, ou mesmo a coluna 'li' com índice b-tree já melhoraria muito.

comment:13 Changed 15 years ago by amuller

Eu disse clausula heaving queria dizer clausula where

comment:14 Changed 15 years ago by rodsouza

Fazer alterações em um software porque o mesmo não tem um bom desempenho por causa da configuração de ambiente é realmente algo imprudente.

Daí quando uma pessoa tenta instalar o ExpressoLivre?, encontra problemas devido ao ambiente e solicita ajuda através de algum canal de comunicação - como o fórum - recebe a resposta "isso não é relativo ao ExpressoLivre?" praticamente um peso e duas medidas.

Mas voltado ao problema de deterioração de código, o exemplo utilizado é apenas um de inúmeros que nem precisa de esforço para serem identificados. Isso fica bem claro na revisão r1261, para ser mais específico as linhas 13 e 14 antes da revisão.

$homedisplay = $GLOBALS['phpgw_info']['user']['preferences']['expressoMail']['mainscreen_showmail']; 
$homedisplay = 'True';

Isso nada mais faz que ratificar o fato que o código foi sendo modificado a esmo e sem nenhuma prudência, isso para não dizer que foi realizado por incipiência.

Essa deteriorização de código criou a situação atual que requer um esforço insano para corrigir problemas gerados pelas alterações feitas sob a justificativa de que isso não é pertinente a um determinado ambiente ou mesmo porque os responsáveis pelos serviços não foram capazes de acompanhar o software.

Em suma, esse assunto já possui diversos tickets inerentes e que sempre possuem a mesma justificativa "isso foi uma situação X".

O fato é que modificações equivocadas foram feitas e para piorar feitas de forma néscia e estulta - o pleonasmo é intencional.

No momento, observando os problemas enfrentados temos em boa parte essas alterações como causadoras - como na revisão citada. Outro exemplo foi a implementação realizada pelo Alexandre Muller que não seria necessária pois procedimento semelhante já existia e foi retirada por algum motivo que apenas os Céus sabem - e para variar o código foi simplesmente limado.

O exemplo utilizado no início do ticket é apenas uma situação e que não é o assunto em questão. Outro exemplo poderia ter sido utilizado, como a descontinuidade do formato das preferências no ExpressoMail? que nessa versão foram retomadas com muito trabalho pelo Alexandre Muller, ou seja, se o código não tivesse sido deteriorado esse esforço não seria necessário.

comment:15 Changed 15 years ago by amuller

A questão da preferência, acho que não é bom exemplo. Outros casos podiam ser exemplificados?

comment:16 Changed 15 years ago by rodsouza

Tá a preferência não foi um exemplo plausível.

Substituindo o exemplo, é a possibilidade da autenticação em outro serviço que não seja o LDAP. Isso era possível e deixou de ser em algum momento.

O fórum demonstra isso http://www.expressolivre.org/html/modules/newbb/viewtopic.php?topic_id=1107&forum=3&post_id=6352

E a opção de autenticação em outros serviços ainda existe na configuração mas não funciona.

comment:17 follow-up: ↓ 18 Changed 15 years ago by niltonneto

Sendo objetivo e direto, pois eu infelizmente não tomei nem sopa de letrinha ontem à noite, muito menos tive tempo de ficar procurando verbetes em dicionários: resumo que essa discussão não está sendo nada produtiva. Esse não é o momento nem de ficar revirando código antigo, nem de ficar rotulando desenvolvedores do projeto como "néscios" e "neófitos", que por sinal, são adjetivos nada agradáveis. Sugiro retomar seus esforços para fecharmos essa fase de testes da nova versão do Expresso. Depois disso, até podemos voltar nesse ticket para avaliarmos melhor a necessidade de um "refactor" da API do Expresso, mas sem denegrir os idealizadores do projeto.

comment:18 in reply to: ↑ 17 Changed 15 years ago by diogenesduarte

Replying to niltonneto:

Sendo objetivo e direto, pois eu infelizmente não tomei nem sopa de letrinha ontem à noite, muito menos tive tempo de ficar procurando verbetes em dicionários: resumo que essa discussão não está sendo nada produtiva. Esse não é o momento nem de ficar revirando código antigo, nem de ficar rotulando desenvolvedores do projeto como "néscios" e "neófitos", que por sinal, são adjetivos nada agradáveis. Sugiro retomar seus esforços para fecharmos essa fase de testes da nova versão do Expresso. Depois disso, até podemos voltar nesse ticket para avaliarmos melhor a necessidade de um "refactor" da API do Expresso, mas sem denegrir os idealizadores do projeto.

Concordo... Já estava a me perguntar qual o problema relatado nesse ticket, pois a essa altura está me parecendo muito mais um desabafo.

comment:19 Changed 15 years ago by rodsouza

Poiseh, é exatamente esse o ponto depois não é possível fazer algo situações que não atendem o interesse do projeto se sobressaem.

Além de que já cansei de ver e ouvir insinuações sobre o desenvolvimento e das funcionalidades como se isso fosse criado atualmente.

Quanto aos termos nada mais são adjetivos - que não precisa de sopa de letrinha pois estão nas colunas diárias de Arnaldo Jabor, Carlos Alberto Sardenberg, Heitor Cony entre outros - que não estão nem de perto comparado aos utilizados nas insinuações, tais como "essa porcaria", "essa merda", "quem foi o idiota" e outros que não vou citar.

Ao passo que as alterações feitas dizem por si basta olhar...

Entretanto se é de interesse empurrar com a barriga não serei eu que nadar contra a maré. E esse ticket foi apenas para documentar a tentativa de evitar um iminente problema que no momento que ocorrer terá este tópico como uma das forma de evitar o que ocorreu anteriormente, ou seja, primeiro se faz de conta que não se tem nada a ver com o problema e depois usa a frase "eu não vi solução para isso" ou ainda "tem que dar nome os bois". Pois esses dois fatos já ficaram bem claros.

E não exatamente um desabafo e sim uma precaução. E o problema são dois a deteriorização e as consequências que isso vem criando e as que ocorrerão em breve.

comment:20 Changed 15 years ago by rodsouza

Ah, esqueci, o ticket atingiu seu objetivo e pode ser finalizado, já que o fato foi exposto e a resposta por mais uma vez foi a mesma utilizada em outros momento, entretando sempre de forma informal, e que agora está escrita e assinada, desta forma cada qual assume seu ponto de vista quando for pertinente.

comment:21 Changed 15 years ago by rodsouza

Desfazendo alterações devido as conclusões.

Committed revision r1292.

comment:22 Changed 15 years ago by wmerlotto

  • Milestone set to Expresso 2.1

comment:23 follow-up: ↓ 24 Changed 14 years ago by wmerlotto

Seria interessante encerrar este ticket e criar outros, um para cada alteração/correção/melhoria... O que acham?

comment:24 in reply to: ↑ 23 Changed 14 years ago by amuller

Replying to wmerlotto:

Seria interessante encerrar este ticket e criar outros, um para cada alteração/correção/melhoria... O que acham?

Para cada o que? Pra mim o ticket era com relação o bloqueio por IP

comment:25 Changed 14 years ago by niltonneto

Pode fechá-lo como inválido. A revisão [1292] desfaz o que foi discutido aqui. Em outro momento, quando fizermos um refactor da API, discutimos então as deteriorizações.

comment:26 Changed 14 years ago by wmerlotto

  • Status changed from new to closed
  • Resolution set to invalid

Tinha entendido que além do bloqueio por IP haviam outros pontos que precisavam ser revistos... Mas blz, fica para o refactor da API (v2.3? :D).

Note: See TracTickets for help on using tickets.