encryptorcode/iam-oauth

View on GitHub

Showing 399 of 407 total issues

Wrong lexicographical order for 'javax.servlet.http.HttpServletResponse' import. Should be before 'org.springframework.web.servlet.handler.HandlerInterceptorAdapter'.
Open

import javax.servlet.http.HttpServletResponse;

Checks that the groups of import declarations appear in the order specifiedby the user. If there is an import but its group is not specified in theconfiguration such an import should be placed at the end of the import list.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Line is longer than 100 characters (found 102).
Open

                    "You are logged in using this authentication framework successfully." + NEW_LINE +

Checks for long lines.

Rationale: Long lines are hard to read in printouts or if developershave limited screen space for the source code, e.g. if the IDEdisplays additional information like project tree, class hierarchy,etc.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Line is longer than 100 characters (found 122).
Open

        return new ModelAndView("redirect:" + AUTHENTICATION_SERVICE.getLoginRedirectPath(request, providerId, redirect));

Checks for long lines.

Rationale: Long lines are hard to read in printouts or if developershave limited screen space for the source code, e.g. if the IDEdisplays additional information like project tree, class hierarchy,etc.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Line is longer than 100 characters (found 112).
Open

     * @param showConsent if true, we need to force the user to accept consent to refresh token is (re)generated

Checks for long lines.

Rationale: Long lines are hard to read in printouts or if developershave limited screen space for the source code, e.g. if the IDEdisplays additional information like project tree, class hierarchy,etc.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Package name 'com.example.simpleServer.config' must match pattern '^[a-z]+(.[a-z][a-z0-9]*)*$'.
Open

package com.example.simpleServer.config;

Checks that package names conform to a specified pattern.

The default value of format for module PackageName has beenchosen to match the requirements in the JavaLanguage specification and the Sun coding conventions. Howeverboth underscores and uppercase letters are rather uncommon, so mostconfigurations should probably assign value ^[a-z]+(\.[a-z][a-z0-9]*)*$ toformat for module PackageName.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

First sentence of Javadoc is missing an ending period.
Open

    /**

Checks thatJavadoc summary sentence does not contain phrases that are not recommended to use.Summaries that contain only the {@inheritDoc} tag are skipped. Check alsoviolate Javadoc that does not contain first sentence.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Class type name 'User' must match pattern '(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)'.
Open

public abstract class BaseSecurityHandler<User extends AUser> extends ASecurityHandler<User> {

Line is longer than 100 characters (found 213).
Open

        } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException | IllegalBlockSizeException | BadPaddingException | InvalidAlgorithmParameterException | ArrayIndexOutOfBoundsException e) {

Checks for long lines.

Rationale: Long lines are hard to read in printouts or if developershave limited screen space for the source code, e.g. if the IDEdisplays additional information like project tree, class hierarchy,etc.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Class type name 'User' must match pattern '(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)'.
Open

public class FileUserHandler<User extends AUser> extends AUserHandler<User> {

First sentence of Javadoc is missing an ending period.
Open

    /**

Checks thatJavadoc summary sentence does not contain phrases that are not recommended to use.Summaries that contain only the {@inheritDoc} tag are skipped. Check alsoviolate Javadoc that does not contain first sentence.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Line is longer than 100 characters (found 105).
Open

    private static final String NEW_LINE = "<br/>\n"; // Adding a \n because I am a fan of curl. Are you?

Checks for long lines.

Rationale: Long lines are hard to read in printouts or if developershave limited screen space for the source code, e.g. if the IDEdisplays additional information like project tree, class hierarchy,etc.

This documentation is written and maintained by the Checkstyle community and is covered under the same license as the Checkstyle project.

Define and throw a dedicated exception instead of using a generic one.
Open

            throw new RuntimeException("Failed to initialize Session service securely", e);

Using such generic exceptions as Error, RuntimeException, Throwable, and Exception prevents calling methods from handling true, system-generated exceptions differently than application-generated errors.

Noncompliant Code Example

public void foo(String bar) throws Throwable {  // Noncompliant
  throw new RuntimeException("My Message");     // Noncompliant
}

Compliant Solution

public void foo(String bar) {
  throw new MyOwnRuntimeException("My Message");
}

Exceptions

Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the analysis).

@Override
public void myMethod() throws Exception {...}

Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.

public void myOtherMethod throws Exception {
  doTheThing();  // this method throws Exception
}

See

Change the visibility of this constructor to "protected".
Open

    public AOauthProviderImpl(

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.

Noncompliant Code Example

public abstract class AbstractClass1 {
    public AbstractClass1 () { // Noncompliant, has public modifier
        // do something here
    }
}

Compliant Solution

public abstract class AbstractClass2 {
    protected AbstractClass2 () {
        // do something here
    }
}

Add a private constructor to hide the implicit public one.
Open

public class DateTimeHandler {

Utility classes, which are collections of static members, are not meant to be instantiated. Even abstract utility classes, which can be extended, should not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

Noncompliant Code Example

class StringUtils { // Noncompliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Compliant Solution

class StringUtils { // Compliant

  private StringUtils() {
    throw new IllegalStateException("Utility class");
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Exceptions

When class contains public static void main(String[] args) method it is not considered as utility class and will be ignored by this rule.

Remove this "clone" implementation; use a copy constructor or copy factory instead.
Open

    public AuthenticationDetail clone() {

Many consider clone and Cloneable broken in Java, largely because the rules for overriding clone are tricky and difficult to get right, according to Joshua Bloch:

Object's clone method is very tricky. It's based on field copies, and it's "extra-linguistic." It creates an object without calling a constructor. There are no guarantees that it preserves the invariants established by the constructors. There have been lots of bugs over the years, both in and outside Sun, stemming from the fact that if you just call super.clone repeatedly up the chain until you have cloned an object, you have a shallow copy of the object. The clone generally shares state with the object being cloned. If that state is mutable, you don't have two independent objects. If you modify one, the other changes as well. And all of a sudden, you get random behavior.

A copy constructor or copy factory should be used instead.

This rule raises an issue when clone is overridden, whether or not Cloneable is implemented.

Noncompliant Code Example

public class MyClass {
  // ...

  public Object clone() { // Noncompliant
    //...
  }
}

Compliant Solution

public class MyClass {
  // ...

  MyClass (MyClass source) {
    //...
  }
}

See

See Also

  • {rule:java:S2157} - "Cloneables" should implement "clone"
  • {rule:java:S1182} - Classes that override "clone" should be "Cloneable" and call "super.clone()"

Define a constant instead of duplicating this literal "expires_in" 3 times.
Open

        if (object.has("access_token") && object.has("expires_in")) {

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

Remove this "clone" implementation; use a copy constructor or copy factory instead.
Open

    public ASession clone() {

Many consider clone and Cloneable broken in Java, largely because the rules for overriding clone are tricky and difficult to get right, according to Joshua Bloch:

Object's clone method is very tricky. It's based on field copies, and it's "extra-linguistic." It creates an object without calling a constructor. There are no guarantees that it preserves the invariants established by the constructors. There have been lots of bugs over the years, both in and outside Sun, stemming from the fact that if you just call super.clone repeatedly up the chain until you have cloned an object, you have a shallow copy of the object. The clone generally shares state with the object being cloned. If that state is mutable, you don't have two independent objects. If you modify one, the other changes as well. And all of a sudden, you get random behavior.

A copy constructor or copy factory should be used instead.

This rule raises an issue when clone is overridden, whether or not Cloneable is implemented.

Noncompliant Code Example

public class MyClass {
  // ...

  public Object clone() { // Noncompliant
    //...
  }
}

Compliant Solution

public class MyClass {
  // ...

  MyClass (MyClass source) {
    //...
  }
}

See

See Also

  • {rule:java:S2157} - "Cloneables" should implement "clone"
  • {rule:java:S1182} - Classes that override "clone" should be "Cloneable" and call "super.clone()"

Rename field "authenticationService"
Open

    private static AuthenticationService authenticationService;

It's confusing to have a class member with the same name (case differences aside) as its enclosing class. This is particularly so when you consider the common practice of naming a class instance for the class itself.

Best practice dictates that any field or member with the same name as the enclosing class be renamed to be more descriptive of the particular aspect of the class it represents or holds.

Noncompliant Code Example

public class Foo {
  private String foo;

  public String getFoo() { }
}

Foo foo = new Foo();
foo.getFoo() // what does this return?

Compliant Solution

public class Foo {
  private String name;

  public String getName() { }
}

//...

Foo foo = new Foo();
foo.getName()

Exceptions

When the type of the field is the containing class and that field is static, no issue is raised to allow singletons named like the type.

public class Foo {
  ...
  private static Foo foo;
  public Foo getInstance() {
    if(foo==null) {
      foo = new Foo();
    }
    return foo;
  }
  ...
}

Change the visibility of this constructor to "protected".
Open

    public JdbcUserHandler(JdbcConfiguration<Session, User> configuration) {

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.

Noncompliant Code Example

public abstract class AbstractClass1 {
    public AbstractClass1 () { // Noncompliant, has public modifier
        // do something here
    }
}

Compliant Solution

public abstract class AbstractClass2 {
    protected AbstractClass2 () {
        // do something here
    }
}

Add a private constructor to hide the implicit public one.
Open

public class FileUtils {

Utility classes, which are collections of static members, are not meant to be instantiated. Even abstract utility classes, which can be extended, should not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

Noncompliant Code Example

class StringUtils { // Noncompliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Compliant Solution

class StringUtils { // Compliant

  private StringUtils() {
    throw new IllegalStateException("Utility class");
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Exceptions

When class contains public static void main(String[] args) method it is not considered as utility class and will be ignored by this rule.

Severity
Category
Status
Source
Language