How can I improve and write more OOP way my username/password validation code

Ruby
#1

require ‘openssl’

class EncryptDecrypt
attr_reader :secreat_key
def initialize(usr, pwd)
@username = usr
@password = pwd
@secreat_key ||= {}
end
def decrypt(usr, pwd)
bool = false
secreat_key.each do |k,v|
decipher = OpenSSL::Cipher::AES.new(128, :CBC)
decipher.decrypt
decipher.key = k
decipher.iv = v.first
plain = decipher.update(v.last) + decipher.final
break bool = true if plain == [usr,pwd].join(’,’)
end
bool
end
def encrypt
data = [@username,@password].join(",")
cipher = OpenSSL::Cipher::AES.new(128, :CBC)
cipher.encrypt
key = cipher.random_key
iv = cipher.random_iv
encrypted = cipher.update(data) + cipher.final
@secreat_key[key] = [iv,encrypted]
end
def user_exist?(usrnam,pswd)
decrypt(usrnam,pswd)
end
def user_add
encrypt
end

private :encrypt, :decrypt
end

user1 = EncryptDecrypt.new(‘arup’,‘xxy’)
user1.user_add
user1.user_exist?(‘arup’,‘xxy’) # => true
user1.user_exist?(‘arup’,‘xx’) # => false

#2

Looks ok.

You should add more comments though.

Also perhaps add the << method to add a user.

user << ‘Joe’

#3

Good morning.

This question is probably easy to answer, as each of us has her/his idea
of ‘more’. Some may even have convictions. I just add ‘comments’,
where I deem them valid. Whether my suggestions are ‘improvements’
depends much on what you try to achieve and who will see your
source-code in the future.

require ‘openssl’

class EncryptDecrypt
attr_reader :secreat_key
def initialize(usr, pwd)
@username = usr
@password = pwd

This ‘assign or not’ is useless. You can straight away assign an empty
array as long as no external actor will access the object’s private
properties. And because the call to ‘initialize()’ will precede most
interaction with such an object…, I really doubt that possibility. But
as my metaprogramming-skills are rudimentary, there can be objections to
all that I write (or anybody else). The way that some people understand
OOP, it can make sense to keep the ‘or’ anyway but I doubt that you have
a use for that kind of code, now.

@secreat_key ||= {}

end
def decrypt(usr, pwd)

Name your variables in a way that helps to guess their utility. For
a variable which is local to the code-block of a method, this ‘rule’ is
certainly of minor importance. However. You should not be obliged to
read all of your code, in a year from now, just because you do not
remember any more…
‘bool’ does not mean a lot.

bool = false
secreat_key.each do |k,v|
  decipher = OpenSSL::Cipher::AES.new(128, :CBC)
  decipher.decrypt
  decipher.key = k
  decipher.iv = v.first
  plain = decipher.update(v.last) + decipher.final
  break bool = true if plain == [usr,pwd].join(',')
end
bool

end
def encrypt
data = [@username,@password].join(",")
cipher = OpenSSL::Cipher::AES.new(128, :CBC)
cipher.encrypt
key = cipher.random_key
iv = cipher.random_iv
encrypted = cipher.update(data) + cipher.final
@secreat_key[key] = [iv,encrypted]
end

user_exist? is an alias for decrypt, so:
alias :user_exist? :encrypt

def user_exist?(usrnam,pswd)
decrypt(usrnam,pswd)
end
def user_add
encrypt
end

private :encrypt, :decrypt

For the alias 'user_exist?, add
public :user_exist?

end

user1 = EncryptDecrypt.new(‘arup’,‘xxy’)
user1.user_add
user1.user_exist?(‘arup’,‘xxy’) # => true
user1.user_exist?(‘arup’,‘xx’) # => false

#4

SORRY for the typo. The alias-declaration must read:
alias :user_exist? :decrypt
You knew that. Anyway.

#5

On Sun, Mar 16, 2014 at 5:00 AM, Arup R. [email protected]
wrote:

class EncryptDecrypt

The class name itself says “violation of the Single Responsibility
Principle”. And why would you expect something with that name to
return a “user” ???

user1 = EncryptDecrypt.new(‘arup’,‘xxy’)

Leaving aside the obvious refactorings of the insides (methods too
long, etc.), I would call this something like a Credential using two
separate classes (Encrypt and Decrypt, though I would probably
also name them as “things” rather than actions, e.g. Encryptor).

credential = Credential.new(‘arup’,‘xxy’)
credential.match?(‘arup’,‘xxy’) # => true

and so on.

Googling ‘ruby and oop’ will find you lots of material to ponder :slight_smile:

HTH,