hash3liZer / evilginx2

Standalone man-in-the-middle attack framework used for phishing login credentials along with session cookies, allowing for the bypass of 2-factor authentication
GNU General Public License v3.0
101 stars 40 forks source link

Remove redundant else clause #15

Closed basebandit closed 3 years ago

basebandit commented 3 years ago

Found a redundant else clause in an if statement that has a return statement.

JamesCullum commented 3 years ago

Can you pull upstream again for the updated test file, so that we can make sure all pass?

basebandit commented 3 years ago

Can you pull upstream again for the updated test file, so that we can make sure all pass?

I have a suggestion, I think the currently failing check after pulling upstream should be in the else clause of the password check like so:

if redditPassword == "" {
        log.Println("[SKIP]", "Valid login is accepted")
    } else {
        _, url, body, _ = test.HttpPost("https://www.localhost/login", baseData+redditPassword)
        test.assertContains(body, "https://www.localhost", "Valid login is accepted")
        test.assertLogContains("all authorization tokens intercepted", "Valid login is detected as correct")
        test.assertEqual(url, "http://example.com/authed", "Redirects to correct page after authentication")
        test.assertLogContains("redirecting to URL: http://example.com/authed", "Redirect to correct page logged")
    }
    test.Clear()

But this check will require for the environment variable REDDITPASSWORD to have been set already.

JamesCullum commented 3 years ago

Hey @basebandit, not sure what you mean exactly. In PRs the variable isn't set (yet?), so that's why the check doesn't work. If you pull upstream again, the check will be skipped.

Edit: Aah I see - more things need to be skipped! Actually all of the stuff later, as they all rely on the successful login. As it fails late and your tests therefore would pass anyways, will merge!

basebandit commented 3 years ago

Hey @basebandit, not sure what you mean exactly. In PRs the variable isn't set (yet?), so that's why the check doesn't work. If you pull upstream again, the check will be skipped.

Edit: Aah I see - more things need to be skipped! Actually all of the stuff later, as they all rely on the successful login. As it fails late and your tests therefore would pass anyways, will merge!

Yes.