Random 5-character token generator

Question

I have an Android app that uses randomly generated tokens to log in. They're 5 characters long, are sent to a user's email address, and have a short lifespan, so I don't think I need anything that's extremely secure.

I'm getting odd result that suggest I'm not really generating random numbers. That, or I'm seeing patterns that don't really exist.

For example, the last few tokens generated were:

• 0pqrH
• UMOPQ
• d789r
• jmnde

These have patterns:

• pqr
• 789
• all caps
• all lowercase

There are some tokens generated with scattered characters:

• f80!#
• NLog#
• Z6kpJ

It just seems that there are too many patterns to be coincidence.

static public String getToken(int chars) {
    String CharSet = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ234567890!@#$";
    String Token = "";
    for (int a = 1; a <= chars; a++) {
        Token += CharSet.charAt(new Random().nextInt(CharSet.length()));
    }
    return Token;
}

Am I generating random numbers correctly? Should I be picking a seed? Is this in my imagination?

Answer 1

This is not entirely only in your imagination

Random objects are meant to be reused. Recreating a Random object each time may cause a specific pattern in the result. This depends a bit on the Java version. There is no need to pick a seed, Java picks a seed for you based on a varying number, and the current system time (see Java 6 source). Newer versions of Java uses a better seedUniquifier method. I am not sure about what implementation the Android platform uses.

Additionally, Java variable names should start with lowercase letters by the coding conventions.

I would also use a StringBuilder for better efficiency and lower memory usage.

To avoid confusion with the Charset class, I would name your String variable chars and your int variable length.

private static final Random random = new Random();
private static final String CHARS = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ234567890!@#$";

public static String getToken(int length) {
    StringBuilder token = new StringBuilder(length);
    for (int i = 0; i < length; i++) {
        token.append(CHARS.charAt(random.nextInt(CHARS.length())));
    }
    return token.toString();
}

Answer 2

It may be important to mention that all random number generators are not truly random. They generate normal distributions of sets over time.

Since the tokens here have a short lifespan, it may not be pertinent, however this should be considered. If you are going for security, overtime someone will be able to figure out the algorithm to crack it, thus be able to generate their own keys.